-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip vetted tests #1091
Draft
mulkieran
wants to merge
3
commits into
stratis-storage:master
Choose a base branch
from
mulkieran:skip-vetted-tests
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Skip vetted tests #1091
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The decorator we want is not one for a 1/0 evironment variable. Instead we want a decorator on every test method and that decorator checks if the test is supposed to be skipped for this run. Signed-off-by: mulhern <[email protected]>
Signed-off-by: mulhern <[email protected]>
Signed-off-by: mulhern <[email protected]>
mulkieran
added a commit
to mulkieran/stratisd
that referenced
this pull request
Aug 5, 2024
They are all failing right now and will continue to do so forever. For future versions, this problem should be addressed with a system that dynamically allows skipping tests once they have been verified as Ok failures. See PR stratis-storage/stratis-cli#1091 for further discussion. Signed-off-by: mulhern <[email protected]>
I wonder if we could use a custom decorator here and avoid the need to pass the test method name explicitly? I.e. using |
mulkieran
added a commit
to mulkieran/stratisd
that referenced
this pull request
Sep 24, 2024
They are all failing right now and will continue to do so forever. For future versions, this problem should be addressed with a system that dynamically allows skipping tests once they have been verified as Ok failures. See PR stratis-storage/stratis-cli#1091 for further discussion. Signed-off-by: mulhern <[email protected]> (cherry picked from commit 11f504c)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The problem I'm addressing is that of not knowing what tests you will want to skip when you are writing the code.
We do some legacy tests on nightly. These legacy tests run older versions of the CLI against the current version of stratisd to try to detect if some failure has crept in. But sometimes, the tests are too precise, and FAIL with the new version of stratisd even when we consider the stratis-cli behavior to be correct. This can happen if, for example, we change the exception that is raised from StratisCliEngineError to StratisCliUserError, that is, we run a pre-check in stratis-cli of what we previously expected the engine to return an error for. In both cases, stratis-cli will exit with an error, and we consider this acceptable.
This approach will put in place a framework that will work for skipping any test that is desired to skip in future. It will be necessary to annotate every test with the skipIf decorator now and for future tests so that this approach can work in future.