Remove #[ignore] tests and GUEST env var, improve test ergonomics
#1196
Conversation
This file contains hidden or 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
ludfjig
force-pushed
the
dedupe_tests
branch
3 times, most recently
from
8314057 to
ef35afa
Compare
ludfjig
force-pushed
the
dedupe_tests
branch
4 times, most recently
from
fc507ba to
d712ead
Compare
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
ludfjig
force-pushed
the
dedupe_tests
branch
from
d712ead to
f92e7fe
Compare
…ent-safe Reduces thread count (20->10) and sandboxes per thread (50->20) so the test can run alongside other tests without exhausting system resources. Removes #[ignore] and the corresponding test-isolated entry in the Justfile. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
ludfjig
force-pushed
the
dedupe_tests
branch
from
f92e7fe to
b288516
Compare
ludfjig
changed the title
#[ignore] tests and GUEST env var, improve test ergonomics
ludfjig
requested review from
danbugs,
dblnz,
devigned,
jprendes,
marosset,
simongdavies and
syntactically
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves Hyperlight’s test ergonomics and CI runtime by removing several #[ignore] tests (making them concurrency-safe), eliminating the GUEST={c,rust} env-var split, and introducing explicit “run with Rust/C/both guests” test helpers so integration tests only need a single run.
Changes:
- Replace
GUEST-based guest selection with explicit helpers (with_rust_*,with_c_*,with_all_*) and update host/integration tests accordingly. - Make previously-isolated tests safe to run in parallel (e.g., tracing interest caching workaround, shared-mem drop test, reduced sandbox stress test size).
- Improve test assertions/output (remove debug prints, add more informative assertion contexts) and adjust guest/test expectations (e.g.,
ExecuteOnHeapmessage).
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/rust_guests/witguest/Cargo.lock | Bumps anyhow lockfile version for witguest test guest. |
| src/tests/rust_guests/simpleguest/src/main.rs | Changes ExecuteOnHeap success string to match updated host test expectations. |
| src/tests/rust_guests/simpleguest/Cargo.lock | Bumps anyhow lockfile version for simpleguest test guest. |
| src/tests/rust_guests/dummyguest/Cargo.lock | Bumps anyhow lockfile version for dummyguest test guest. |
| src/hyperlight_testing/src/tracing_subscriber.rs | Adds callsite Interest handling and span retrieval helper to stabilize parallel tracing tests. |
| src/hyperlight_host/tests/sandbox_host_tests.rs | Migrates tests to explicit guest helpers; removes per-test guest env handling/prints. |
| src/hyperlight_host/tests/integration_test.rs | Migrates integration tests to explicit guest helpers; makes execute_on_heap non-ignored and tests both feature paths. |
| src/hyperlight_host/tests/common/mod.rs | Introduces new sandbox helper APIs (rust-only, c-only, both). |
| src/hyperlight_host/src/testing/log_values.rs | Removes unused build-metadata testing helper module. |
| src/hyperlight_host/src/sandbox/uninitialized.rs | Makes test_trace_trace parallel-safe and removes #[ignore] by addressing tracing interest caching. |
| src/hyperlight_host/src/sandbox/initialized_multi_use.rs | Reduces sandbox creation stress test to 200 and removes #[ignore]. |
| src/hyperlight_host/src/mem/shared_mem.rs | Makes shared-mem drop test concurrency-safe and removes #[ignore] (Linux only, not miri). |
| src/hyperlight_host/src/mem/layout.rs | Moves/introduces max-memory test coverage at the layout layer. |
| Justfile | Removes duplicated integration test runs and updates test recipes to the new flow. |
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Use Interest::sometimes() in TracingSubscriber to prevent the global interest cache from permanently caching Interest::never() when another thread's no-op subscriber registers callsites first. Add a warmup call + rebuild_interest_cache() to ensure callsites are properly registered before the real test run. Also removes the now-unused build_metadata_testing module and TestLogger dependency. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Replace the GUEST environment variable approach with explicit helper functions (with_rust_sandbox, with_c_sandbox, with_all_sandboxes, etc.) that make test intent clearer and remove runtime environment coupling. Each test now explicitly declares which guest(s) it needs. Tests that work with both guests use with_all_sandboxes to run against both Rust and C guests in a single test invocation, removing the need for separate test-integration runs per guest language. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Move the test from integration tests into the layout module where it belongs, and rewrite it to test SandboxMemoryLayout::get_memory_size directly instead of going through UninitializedSandbox::new. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
…tests
Adds descriptive error messages to pattern-matching assertions so test failures show the actual value. Removes println!("{:?}", res) calls that were only useful for manual debugging.
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
ludfjig
force-pushed
the
dedupe_tests
branch
from
b288516 to
89e52f7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Reduces
build-testCI step from around 20min to 10min!Makes several previously-isolated tests concurrent-safe so they no longer need
#[ignore]and separate CI runs:test_drop: use unique mapping size instead of address-based checkscreate_1000_sandboxes: reduce to 200 sandboxes, rename accordinglyexecute_on_heap: remove#[ignore], test both feature paths inlinetest_trace_trace: fixInterestcaching race withInterest::sometimes()and callsite warmupReplaces the
GUEST={c,rust}env var with explicit helpers (with_rust_sandbox,with_c_sandbox,with_all_sandboxes), so each test declares which guest(s) it needs andtest-integrationonly runs once. This reduces CI time by alot, since we're not accidentally running some tests twice (wit-tests, etc)Also improves
assert!(matches!())calls with error context and removes debugprintln!.Removes need for speciying GUEST={c,rust} when running integratiton tests. This will deduplicate a bunch of tests injust testand therefore CI, since we are currently accidentally running a bunch of tests that don't rely on c/rust guests twice for no reason.Certain tests still require #[ignore] (logging tests) but hopefully these can be removed soon as well. Please review PR commit-by-commit.
Todo in future PR: remove more duplicate tests when running tracing tests (runs all tests AGAIN, which is not needed)