-
Notifications
You must be signed in to change notification settings - Fork 7
QA-487: feat: Split out cross-platform tests #80
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
QA-487: feat: Split out cross-platform tests #80
Conversation
10a5623 to
78493c3
Compare
4a499a4 to
78493c3
Compare
ghost
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
All comments can be easily fixed, and do not require re-review, so pre-approving.
tests/utils/fixtures/fixtures.py
Outdated
| @pytest.fixture(scope="function", autouse=True) | ||
| def cross_platform_test(request): | ||
| mark = request.node.get_closest_marker("cross_platform") | ||
| option = request.config.getoption("--cross-platform-tests") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about inverting the option (--no-cross-platform-tests) and passing it in mender-qa? Otherwise we might omit tests by accident, and running tests locally will skip the tests for seemingly no reason (the message is usually not displayed unless special options are passed). It also makes it more explicit that the mender-qa job is omitting certain tests, which I think is more important to know for a reader than the opposite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point that I have also been considering (rather, I went back and forth with it when I first implemented two selectors for "software" and "platform" tests).
With the current implementation, as you said, it is obscure that the default execution will skip a bunch of tests. With your suggestion, I then think we would need to flags: --no-cross-platform-tests and --only-cross-platform-tests. This way we can clearly and explicitly select one or the other, while still be default running all. Then just adjust mender-qa accordingly 👍
Once you commented on it, and I also had doubts around it, I will then go with this approach!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworked now.
|
@kacf Thank you for the test-by-best review here and in mendersoftware/meta-mender#1884 - there were few that I was unsure but I trusted I would get a proper review here! I'll rework now the three PRs together with the change of |
|
@lluiscampos what about: |
@oleorhagen With my original criteria, likely an oversight. However taking into account @kacf comment here, this test is verifying boot loader integration also so I think we should keep it. |
78493c3 to
c8548a3
Compare
Signed-off-by: Lluis Campos <[email protected]>
Use a marker and two command line options to mark and select/deselect tests that are platform agnostic. The default behavior will be the same (aka run all tests), while the user can optionally skipping the platform agnostic tests or run them exclusively. Mark then all tests accordingly. Along the way, minor tests doc improvements have been made. Changelog: None Ticket: QA-487 Signed-off-by: Lluis Campos <[email protected]>
c8548a3 to
30ae44e
Compare
Uh oh!
There was an error while loading. Please reload this page.