flowey: vmm_tests: introduce step to verify all tests run on at least one runner #2094
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces infrastructure to verify test coverage by creating a step that parses nextest JUnit XML files to collect test names and ensures all tests run on at least one runner. The implementation adds nextest list command generation, JSON artifact publishing, and a new verification node for comparing test results.
- Adds a new verification step to parse JUnit XML files and extract test names
- Extends nextest integration to generate list commands and publish JSON artifacts
- Updates test result publishing to include nextest list JSON output
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| flowey/flowey_lib_hvlite/src/_jobs/verify_all_tests_run.rs | New node that parses JUnit XML files to extract test names for verification |
| flowey/flowey_lib_common/src/run_cargo_nextest_run.rs | Extended to generate nextest list commands and capture JSON output |
| flowey/flowey_lib_common/src/gen_cargo_nextest_list_cmd.rs | New module for generating cargo nextest list commands |
| flowey/flowey_lib_common/src/publish_test_results.rs | Updated to publish nextest list JSON as artifacts |
| .github/workflows/openvmm-pr.yaml | Generated workflow updates reflecting the new artifact publishing |
| .github/workflows/openvmm-pr-release.yaml | Generated workflow updates reflecting the new artifact publishing |
damanm24
changed the title
|
Very cool idea, but I think it needs a bunch of refactoring to clean up the cases where steps have been combined that really should be separate. |
flowey/flowey_lib_hvlite/src/_jobs/consume_and_test_nextest_unit_tests_archive.rs
Outdated
Show resolved
Hide resolved
flowey/flowey_lib_hvlite/src/_jobs/consume_and_test_nextest_vmm_tests_archive.rs
Outdated
Show resolved
Hide resolved
| fn get_nextest_list_output_from_stdout(output: &str) -> anyhow::Result<serde_json::Value> { | ||
| // nextest list prints a few lines of non-json output before the actual | ||
| // JSON output, so we need to find the first line that is valid JSON | ||
| for line in output.lines() { |
There was a problem hiding this comment.
Is it always the same number of lines? Could we just skip that many?
| let parse = ctx.emit_rust_step( | ||
| "parse and analyze junit logs and nextest list output", | ||
| |ctx| { | ||
| // This step takes all of the junit XML files (i.e. the tests that were run) and the nextest list output (i.e. the tests that were built) |
There was a problem hiding this comment.
Just thinking through the logic here. When we run nextest list I think that's going to use the same filtering that nextest run does. So I would expect the outputs to always match if we run list on the test runners. I think the correct approach would be to run nextest list on each vmm test builder with the include-ignored flag to get a complete list of everything built from that platform. Then combine that with the lists of everything run on the different runners. Thoughts?
There was a problem hiding this comment.
Additionally, have you tried causing a failure here by manually tweaking something to verify that the logic is working?
There was a problem hiding this comment.
I do think running list on the builders instead of the runners is the way to go, with the include ignored flag i'd expect the runners to all output the same lists for their platform. Moving to the builder just dedups it.
2 participants
This PR resolves #2010
With #1991 it is possible (however unlikely) for a contributor to introduce a test that doesn't end up running on any of the vmm_test jobs in CI. This PR introduces a step that verifies the list of all tests built as part of a CI run are run on at least one vmm_test job.
Here are a list of changes that make this possible:
nextest listcommand which dumps the tests that are built as part of the nextest archive.nextest listand the corresponding junit xml that was produced from all vmm_test runs. This node then gathers a full set of tests from both artifacts and compares them to make sure the sets are equal.