-
Notifications
You must be signed in to change notification settings - Fork 5
xen: Add test-ring0 tests #304
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
Conversation
8fcffa8
to
b4ce123
Compare
LGTM. As this adds a reboot to the xen test, we need to test it in CI conditions to see how it behaves, before merging. CC @gduperrey |
8.2 xen ✅ |
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.
A few cosmetic remarks, otherwise lgtm
2025-06-19:
|
tests/xen/test_ring0.py
Outdated
def host_with_ring0_tests(host_with_saved_yum_state: Host) \ | ||
-> Generator[Host, None, None]: |
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.
Trailing backslashes are discouraged when possible in PEP8: https://peps.python.org/pep-0008/#maximum-line-length
Adding the newline before the closing parenthesis seems to be quite usual.
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.
On the contrary, this style is used just about everywhere in our 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.
Well, we're only at the start of using type hints, and I see just a few occurrences of that pattern. I don't see a reason to select a style that does not conform to pep8 here, and would rather prefer that we adjust those few ones :)
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.
Applied formatter.
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.
well, the additional newline may not be necessary, but I can live with it :)
Signed-off-by: Tu Dinh <[email protected]>
Run the test-ring0 tests that are straightforward to run.
The ones that are not (e.g. legacy tests, tests that crash the host or leak resources infinitely) are skipped, but included for completeness.
test_xst_memory_leak
requires a custom kernel withCONFIG_DEBUG_KMEMLEAK
set.