Tests: create new fixtures to make writing booted/restored simpler #4934
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
pb8o
added
Priority: Low
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4934 +/- ##
=======================================
Coverage 84.07% 84.07%
=======================================
Files 251 251
Lines 28059 28059
=======================================
Hits 23592 23592
Misses 4467 4467
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pb8o
force-pushed
the
test-refactors
branch
10 times, most recently
from
ef74727 to
0ba22d7
Compare
zulinx86
reviewed
Dec 3, 2024
tests/integration_tests/functional/test_cpu_features_aarch64.py
Outdated
Show resolved
Hide resolved
There is some information in the Microvm class that we don't save in the snapshot. Some tests do depend on those, so to make the booted/restored case homogenous, make room in the snapshot to save metadata that we can then restore. Signed-off-by: Pablo Barbáchano <[email protected]>
pb8o
force-pushed
the
test-refactors
branch
2 times, most recently
from
3432d94 to
ae90872
Compare
pb8o
added
Status: Awaiting review
zulinx86
approved these changes
Dec 4, 2024
There was a problem hiding this comment.
Looks good to me to merge :)
Do you think it's worth updating the description for pytest fixtures in the test README file?
Lines 300 to 308 in 81bae9f
|
It'd be nice to run it in non-PR mode before merging. |
zulinx86
requested changes
Dec 4, 2024
5 tasks
zulinx86
previously approved these changes
Dec 5, 2024
I think at least the latest I have run it in both modes. |
Manciukic
previously approved these changes
Dec 5, 2024
tests/integration_tests/functional/test_cpu_features_aarch64.py
Outdated
Show resolved
Hide resolved
pb8o
force-pushed
the
test-refactors
branch
2 times, most recently
from
5f8bd1a to
c1aecb7
Compare
roypat
reviewed
Dec 5, 2024
tests/integration_tests/functional/test_cpu_features_aarch64.py
Outdated
Show resolved
Hide resolved
pb8o
force-pushed
the
test-refactors
branch
2 times, most recently
from
714c78d to
2abf1da
Compare
Some further simplifications: - Simplify the "_host" tests as they will provide the same result in both A/B situations. - Add a second MicrovmFactory fixture with the "A" firecracker. - Add a uvm_any fixture that includes all CPU templates and also a boot/restored dimension. This decreases the need for separate tests. For example the guest test test_check_vulnerability_files_ab can now test all variants: 2 (restored/booted) * 3 (kernels) * 9 (cpu templates) = 54 tests Signed-off-by: Pablo Barbáchano <[email protected]>
Use a new uvm_any_booted fixture to make the test simpler. Signed-off-by: Pablo Barbáchano <[email protected]>
Use the new uvm_any fixture to make the tests simpler Signed-off-by: Pablo Barbáchano <[email protected]>
All tests in the file are x86_64 specific, so do the skip at the top-level, once. Signed-off-by: Pablo Barbáchano <[email protected]>
Make the common feature flags to get some clarity on what are the differences between the CPUs. Signed-off-by: Pablo Barbáchano <[email protected]>
This is the same test spread over two files. Putting it together in a new file so they don't drift over time. Signed-off-by: Pablo Barbáchano <[email protected]>
This is a fairly common repeated pattern, so extract it into a method to make writing tests simpler. Signed-off-by: Pablo Barbáchano <[email protected]>
Rewrite test_rng using uvm_any. Signed-off-by: Pablo Barbáchano <[email protected]>
This fixture is not used anymore, as it is mostly superseded by cpu_template_any. Signed-off-by: Pablo Barbáchano <[email protected]>
Since we only have one rootfs, simplify the fixture to return a single value. This will have also not show it as part of the test instance name. Signed-off-by: Pablo Barbáchano <[email protected]>
Add a parameter so that we can control vcpu number and memory. Right now it uses a hardcoded value, but it can be overridden per test. Signed-off-by: Pablo Barbáchano <[email protected]>
roypat
approved these changes
Dec 5, 2024
Manciukic
approved these changes
Dec 5, 2024
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.
Changes
This introduces a few new pytest fixtures to deal with CPU template differences, A/B tests, and booted/restored VMs.
In addition there's some cleanups in the affected tests.
Reason
Making those tests simple to write is important so that we don't forget one case. In addition they become easier to write.
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.