Skip to content
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
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mulkieran
Copy link
Member

@mulkieran mulkieran commented Aug 5, 2024

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.

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 <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran mulkieran self-assigned this Aug 5, 2024
Signed-off-by: mulhern <amulhern@redhat.com>
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 <amulhern@redhat.com>
@bmr-cymru
Copy link
Member

I wonder if we could use a custom decorator here and avoid the need to pass the test method name explicitly? I.e. using __name__ from the decorator and then passing the result to skipIf? I haven't tried this out but it might allow us to avoid the need to pass the name and "skip requested" string explicitly. It's a small saving but if it works out it might be worth it.

@mulkieran
Copy link
Member Author

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 <amulhern@redhat.com>
(cherry picked from commit 11f504c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Pending
Development

Successfully merging this pull request may close these issues.

2 participants