Remove #[ignore] tests and GUEST env var, improve test ergonomics (#1196)

* Make test_drop able to run concurrently

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

* Rename create_1000_sandboxes to create_200_sandboxes and make concurrent-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>

* Allow execute_on_heap to execute concurrently

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

* Make test_trace_trace concurrent-safe

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>

* Remove GUEST env var and refactor test helpers

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 max_memory_sandbox test to layout module

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>

* Add error context to assert!(matches!()) and remove debug println in 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>

---------

Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>

Author: ludfjig

Justfile

@@ -205,30 +205,27 @@ like-ci config=default-target hypervisor="kvm":
     just check-license-headers
 
 # runs all tests
-test target=default-target features="": (test-unit target features) (test-isolated target features) (test-integration "rust" target features) (test-integration "c" target features) (test-doc target features)
+test target=default-target features="": (test-unit target features) (test-isolated target features) (test-integration target features) (test-doc target features)
 
 # runs unit tests
 test-unit target=default-target features="":
     {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} --lib
 
 # runs tests that requires being run separately, for example due to global state
 test-isolated target=default-target features="" :
-    {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::uninitialized::tests::test_trace_trace --exact --ignored
     {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::uninitialized::tests::test_log_trace --exact --ignored
-    {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::initialized_multi_use::tests::create_1000_sandboxes --exact --ignored
     {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::outb::tests::test_log_outb_log --exact --ignored
-    {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- mem::shared_mem::tests::test_drop --exact --ignored
     {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --test integration_test -- log_message --exact --ignored
     @# metrics tests
     {{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F function_call_metrics,init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- metrics::tests::test_metrics_are_emitted --exact 
-# runs integration tests. Guest can either be "rust" or "c"
-test-integration guest target=default-target features="":
-    @# run execute_on_heap test with feature "executable_heap" on and off
-    {{if os() == "windows" { "$env:" } else { "" } }}GUEST="{{guest}}"{{if os() == "windows" { ";" } else { "" } }} {{ cargo-cmd }} test --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} --test integration_test execute_on_heap {{ if features =="" {" --features executable_heap"} else {"--features executable_heap," + features} }} -- --ignored
-    {{if os() == "windows" { "$env:" } else { "" } }}GUEST="{{guest}}"{{if os() == "windows" { ";" } else { "" } }} {{ cargo-cmd }} test --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} --test integration_test execute_on_heap {{ if features =="" {""} else {"--features " + features} }} -- --ignored
+
+# runs integration tests
+test-integration target=default-target features="":
+    @# run execute_on_heap test with feature "executable_heap" on (runs with off during normal tests)
+    {{ cargo-cmd }} test {{ if features =="" {"--features executable_heap"} else if features=="no-default-features" {"--no-default-features --features executable_heap"} else {"--no-default-features -F init-paging,executable_heap," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} --test integration_test execute_on_heap
 
     @# run the rest of the integration tests
-    {{if os() == "windows" { "$env:" } else { "" } }}GUEST="{{guest}}"{{if os() == "windows" { ";" } else { "" } }} {{ cargo-cmd }} test -p hyperlight-host {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} --test '*'
+    {{ cargo-cmd }} test -p hyperlight-host {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} --test '*'
 
 # tests compilation with no default features on different platforms
 test-compilation-no-default-features target=default-target:

src/hyperlight_host/src/mem/layout.rs

@@ -662,4 +662,13 @@ mod tests {
             get_expected_memory_size(&sbox_mem_layout)
         );
     }
+
+    #[test]
+    fn test_max_memory_sandbox() {
+        let mut cfg = SandboxConfiguration::default();
+        cfg.set_input_data_size(0x40000000);
+        let layout = SandboxMemoryLayout::new(cfg, 4096, 2048, 4096, 4096, None).unwrap();
+        let result = layout.get_memory_size();
+        assert!(matches!(result.unwrap_err(), MemoryRequestTooBig(..)));
+    }
 }

src/hyperlight_host/src/mem/shared_mem.rs

@@ -1333,50 +1333,61 @@ mod tests {
         assert_eq!(data, ret_vec);
     }
 
-    /// A test to ensure that, if a `SharedMem` instance is cloned
-    /// and _all_ clones are dropped, the memory region will no longer
-    /// be valid.
-    ///
-    /// This test is ignored because it is incompatible with other tests as
-    /// they may be allocating memory at the same time.
-    ///
-    /// Marking this test as ignored means that running `cargo test` will not
-    /// run it. This feature will allow a developer who runs that command
-    /// from their workstation to be successful without needing to know about
-    /// test interdependencies. This test will, however, be run explicitly as a
-    /// part of the CI pipeline.
+    /// Test that verifies memory is properly unmapped when all SharedMemory
+    /// references are dropped.
     #[test]
-    #[ignore]
-    #[cfg(target_os = "linux")]
+    #[cfg(all(target_os = "linux", not(miri)))]
     fn test_drop() {
-        use proc_maps::maps_contain_addr;
+        use proc_maps::get_process_maps;
+
+        // Use a unique size that no other test uses to avoid false positives
+        // from concurrent tests allocating at the same address.
+        // The mprotect calls split the mapping into 3 regions (guard, usable, guard),
+        // so we check for the usable region which has this exact size.
+        //
+        // NOTE: If this test fails intermittently, there may be a race condition
+        // where another test allocates memory at the same address between our
+        // drop and the mapping check. Ensure UNIQUE_SIZE is not used by any
+        // other test in the codebase to avoid this.
+        const UNIQUE_SIZE: usize = PAGE_SIZE_USIZE * 17;
 
         let pid = std::process::id();
 
-        let eshm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap();
+        let eshm = ExclusiveSharedMemory::new(UNIQUE_SIZE).unwrap();
         let (hshm1, gshm) = eshm.build();
         let hshm2 = hshm1.clone();
-        let addr = hshm1.raw_ptr() as usize;
 
-        // ensure the address is in the process's virtual memory
-        let maps_before_drop = proc_maps::get_process_maps(pid.try_into().unwrap()).unwrap();
+        // Use the usable memory region (not raw), since mprotect splits the mapping
+        let base_ptr = hshm1.base_ptr() as usize;
+        let mem_size = hshm1.mem_size();
+
+        // Helper to check if exact mapping exists (matching both address and size)
+        let has_exact_mapping = |ptr: usize, size: usize| -> bool {
+            get_process_maps(pid.try_into().unwrap())
+                .unwrap()
+                .iter()
+                .any(|m| m.start() == ptr && m.size() == size)
+        };
+
+        // Verify mapping exists before drop
         assert!(
-            maps_contain_addr(addr, &maps_before_drop),
-            "shared memory address {:#x} was not found in process map, but should be",
-            addr,
+            has_exact_mapping(base_ptr, mem_size),
+            "shared memory mapping not found at {:#x} with size {}",
+            base_ptr,
+            mem_size
         );
-        // drop both shared memory instances, which should result
-        // in freeing the memory region
+
+        // Drop all references
         drop(hshm1);
         drop(hshm2);
         drop(gshm);
 
-        let maps_after_drop = proc_maps::get_process_maps(pid.try_into().unwrap()).unwrap();
-        // now, ensure the address is not in the process's virtual memory
+        // Verify exact mapping is gone
         assert!(
-            !maps_contain_addr(addr, &maps_after_drop),
-            "shared memory address {:#x} was found in the process map, but shouldn't be",
-            addr
+            !has_exact_mapping(base_ptr, mem_size),
+            "shared memory mapping still exists at {:#x} with size {} after drop",
+            base_ptr,
+            mem_size
         );
     }
 

src/hyperlight_host/src/sandbox/initialized_multi_use.rs

@@ -1119,41 +1119,38 @@ mod tests {
     }
 
     #[test]
-    #[ignore] // this test runs by itself because it uses a lot of system resources
-    fn create_1000_sandboxes() {
-        let barrier = Arc::new(Barrier::new(21));
+    fn create_200_sandboxes() {
+        const NUM_THREADS: usize = 10;
+        const SANDBOXES_PER_THREAD: usize = 20;
 
-        let mut handles = vec![];
+        // barrier to make sure all threads start their work simultaneously
+        let start_barrier = Arc::new(Barrier::new(NUM_THREADS + 1));
+        let mut thread_handles = vec![];
 
-        for _ in 0..20 {
-            let c = barrier.clone();
+        for _ in 0..NUM_THREADS {
+            let barrier = start_barrier.clone();
 
             let handle = thread::spawn(move || {
-                c.wait();
-
-                for _ in 0..50 {
-                    let usbox = UninitializedSandbox::new(
-                        GuestBinary::FilePath(
-                            simple_guest_as_string().expect("Guest Binary Missing"),
-                        ),
-                        None,
-                    )
-                    .unwrap();
+                barrier.wait();
 
-                    let mut multi_use_sandbox: MultiUseSandbox = usbox.evolve().unwrap();
+                for _ in 0..SANDBOXES_PER_THREAD {
+                    let guest_path = simple_guest_as_string().expect("Guest Binary Missing");
+                    let uninit =
+                        UninitializedSandbox::new(GuestBinary::FilePath(guest_path), None).unwrap();
 
-                    let res: i32 = multi_use_sandbox.call("GetStatic", ()).unwrap();
+                    let mut sandbox: MultiUseSandbox = uninit.evolve().unwrap();
 
-                    assert_eq!(res, 0);
+                    let result: i32 = sandbox.call("GetStatic", ()).unwrap();
+                    assert_eq!(result, 0);
                 }
             });
 
-            handles.push(handle);
+            thread_handles.push(handle);
         }
 
-        barrier.wait();
+        start_barrier.wait();
 
-        for handle in handles {
+        for handle in thread_handles {
             handle.join().unwrap();
         }
     }