Skip to content

petri: Refactor test construction to own the async pool - #4180

Open
Steven Malis (smalis-msft) wants to merge 2 commits into
microsoft:mainfrom
smalis-msft:petri-own-async
Open

petri: Refactor test construction to own the async pool#4180
Steven Malis (smalis-msft) wants to merge 2 commits into
microsoft:mainfrom
smalis-msft:petri-own-async

Conversation

@smalis-msft

@smalis-msft Steven Malis (smalis-msft) commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Refactor test construction in petri so that petri itself owns the DefaultPool used to run async tests, rather than the test owning it. This will enable future diagnostics that need to be run by the pool after the test has failed or panicked.

I recommend reviewing with whitespace diffs hidden, a number of code blocks lost a level of indentation but have no other change.

@smalis-msft
Steven Malis (smalis-msft) requested a review from a team as a code owner August 6, 2026 22:36
Copilot AI lite review requested due to automatic review settings August 6, 2026 22:36
@smalis-msft Steven Malis (smalis-msft) added the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Aug 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Refactors the petri test harness so petri owns the async executor pool (via DefaultPool) for async tests, instead of each test constructing/running its own pool—enabling post-failure/panic diagnostics to run on the same pool.

Changes:

  • Introduces async-oriented test construction (SimpleTest::new_async) and new test_sync!/unstable_test_sync! macros for synchronous tests.
  • Updates existing tests to receive DefaultDriver from petri instead of calling DefaultPool::run_with themselves.
  • Makes IoPool::named public to support naming task pools for tracing.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
vmm_tests/vmm_tests/tests/tests/ttrpc/fd_passing.rs Converts test to async + driver-injected execution (removes local pool ownership).
vmm_tests/vmm_tests/tests/tests/ttrpc.rs Converts ttrpc tests to async + driver-injected execution (removes local pool ownership).
vmm_tests/vmm_tests/tests/cca.rs Switches a sync test registration to test_sync!.
vmm_tests/vmm_test_macros/src/lib.rs Updates macro expansion to build async tests via SimpleTest::new_async.
tmk/tmk_tests/src/lib.rs Switches host TMK tests to test_sync! and refactors async pool usage out of test bodies.
support/pal/pal_async/src/io_pool.rs Exposes IoPool::named publicly for named executors in traces.
petri/src/test.rs Adds SimpleTest::new_async, adds sync test macros, and renames SimpleTest::new to new_sync.

Comment thread petri/src/test.rs
Comment thread petri/src/test.rs
Comment thread petri/src/test.rs Outdated
Comment thread petri/src/test.rs
Comment thread petri/src/test.rs Outdated
Comment thread vmm_tests/vmm_tests/tests/tests/ttrpc.rs
Copilot AI review requested due to automatic review settings August 6, 2026 23:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

tmk/tmk_tests/src/lib.rs:273

  • The Hyper-V variant previously applied modify_backend(|b| b.with_allow_early_vtl0_access(true)) (with a TODO explaining it). This refactor removed that call only for the Hyper-V path, which makes backend behavior inconsistent and may reintroduce the underlying “early access” issue on Windows. Either restore the same backend modification here or add a short comment explaining why Hyper-V no longer needs it while OpenVMM still does.
        let logger = params.logger.clone();
        let mut vm = petri::PetriVmBuilder::new(params, artifacts.vm, &driver)?
            .with_openhcl_command_line(OPENHCL_COMMAND_LINE)
            .with_expect_no_boot_event()
            .with_openhcl_agent_file("tmk_vmm", artifacts.tmk_vmm)
            .with_openhcl_agent_file("simple_tmk", artifacts.tmk)
            .with_processor_topology(ProcessorTopology {
                vp_count: 1,
                ..Default::default()
            })
            .run_without_agent()
            .await?;

Comment thread petri/src/test.rs
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@smalis-msft Steven Malis (smalis-msft) removed the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants