Skip to content

Conversation

@zebreus
Copy link
Contributor

@zebreus zebreus commented Jan 14, 2026

Description

Copilot AI review requested due to automatic review settings January 14, 2026 16:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR revamps the WASIX test infrastructure by introducing a new test utility framework and comprehensive context switching tests. The changes replace the old inline test compilation approach with a more structured system using build scripts and test utilities.

Changes:

  • Added wasixcc_test_utils.rs providing utilities for compiling and running WASIX tests
  • Refactored context_switching.rs to use the new build script-based test approach
  • Added 19 new context switching test cases with corresponding C source files and build scripts
  • Updated .gitignore to exclude build artifacts while preserving source files

Reviewed changes

Copilot reviewed 57 out of 76 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
lib/wasix/tests/wasixcc_test_utils.rs New test utility framework with compilation and execution helpers
lib/wasix/tests/context_switching.rs Refactored to use build scripts instead of inline compilation
lib/wasix/tests/.gitignore Updated patterns for new test structure
lib/wasix/tests/context_switching/*/build.sh Simple build scripts for each test case
lib/wasix/tests/context_switching//.c Comprehensive test cases for context switching scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +304 to +330
pub fn compile_shared_library(
&self,
sources: &[&str],
lib_name: &str,
extra_flags: &[&str],
) -> Result<PathBuf, anyhow::Error> {
let cpp = sources
.iter()
.find(|source| {
PathBuf::from(source)
.extension()
.map_or(false, |ext| ext == "cpp" || ext == "cc")
})
.is_some();

let link_dir_flag = format!("-L{}", self.output_dir.display());
let mut base_flags = vec![
"-fwasm-exceptions",
"-shared",
"-fPIC",
link_dir_flag.as_str(),
];
base_flags.extend_from_slice(extra_flags);

self.wasixcc(sources, &self.default_executable, &base_flags, &[], cpp)?;

Ok(self.default_executable.clone())
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The lib_name parameter is unused. The function always outputs to self.default_executable regardless of the lib_name parameter value, which could lead to unexpected behavior where multiple shared libraries would overwrite each other.

Copilot uses AI. Check for mistakes.
Comment on lines 8 to 156
fn test_simple_switching() {
test_with_wasixcc("simple_switching").unwrap();
let wasm_path = run_build_script(file!(), "simple_switching").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_switching_with_main() {
test_with_wasixcc("switching_with_main").unwrap();
let wasm_path = run_build_script(file!(), "switching_with_main").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_switching_to_a_deleted_context() {
test_with_wasixcc("switching_to_a_deleted_context").unwrap();
let wasm_path = run_build_script(file!(), "switching_to_a_deleted_context").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_switching_threads() {
test_with_wasixcc("switching_in_threads").unwrap();
let wasm_path = run_build_script(file!(), "switching_in_threads").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_multiple_contexts() {
test_with_wasixcc("multiple_contexts").unwrap();
let wasm_path = run_build_script(file!(), "multiple_contexts").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_error_handling() {
test_with_wasixcc("error_handling").unwrap();
let wasm_path = run_build_script(file!(), "error_handling").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_nested_switches() {
test_with_wasixcc("nested_switches").unwrap();
let wasm_path = run_build_script(file!(), "nested_switches").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_contexts_with_mutexes() {
test_with_wasixcc("contexts_with_mutexes").unwrap();
let wasm_path = run_build_script(file!(), "contexts_with_mutexes").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_contexts_with_env_vars() {
test_with_wasixcc("contexts_with_env_vars").unwrap();
let wasm_path = run_build_script(file!(), "contexts_with_env_vars").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_contexts_with_signals() {
test_with_wasixcc("contexts_with_signals").unwrap();
let wasm_path = run_build_script(file!(), "contexts_with_signals").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_contexts_with_timers() {
test_with_wasixcc("contexts_with_timers").unwrap();
let wasm_path = run_build_script(file!(), "contexts_with_timers").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_contexts_with_pipes() {
test_with_wasixcc("contexts_with_pipes").unwrap();
let wasm_path = run_build_script(file!(), "contexts_with_pipes").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_pending_file_operations() {
test_with_wasixcc("pending_file_operations").unwrap();
let wasm_path = run_build_script(file!(), "pending_file_operations").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_recursive_host_calls() {
test_with_wasixcc("recursive_host_calls").unwrap();
let wasm_path = run_build_script(file!(), "recursive_host_calls").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_malloc_during_switch() {
test_with_wasixcc("malloc_during_switch").unwrap();
let wasm_path = run_build_script(file!(), "malloc_during_switch").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_nested_host_call_switch() {
test_with_wasixcc("nested_host_call_switch").unwrap();
let wasm_path = run_build_script(file!(), "nested_host_call_switch").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_switch_to_never_resumed() {
test_with_wasixcc("switch_to_never_resumed").unwrap();
let wasm_path = run_build_script(file!(), "switch_to_never_resumed").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_three_way_recursion() {
test_with_wasixcc("three_way_recursion").unwrap();
let wasm_path = run_build_script(file!(), "three_way_recursion").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}

#[cfg(all(unix, not(target_os = "macos")))]
#[test]
fn test_contexts_with_setjmp() {
test_with_wasixcc("contexts_with_setjmp").unwrap();
let wasm_path = run_build_script(file!(), "contexts_with_setjmp").unwrap();
let test_dir = wasm_path.parent().unwrap();
run_wasm(&wasm_path, test_dir).unwrap();
}
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Repetitive code pattern across all test functions. Each test function follows the exact same three-line pattern. Consider creating a helper macro or function to reduce code duplication, which would improve maintainability and reduce the chance of copy-paste errors.

Copilot uses AI. Check for mistakes.

impl<B: Write + Send + Debug + Unpin + 'static> CaptureFile<B> {
fn new(buffer: Arc<Mutex<Vec<u8>>>, file: Option<B>) -> Self {
Self { buffer, file: file }
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Redundant field initialization. The field 'file' can be directly assigned the value without explicitly naming it again.

Copilot uses AI. Check for mistakes.
Comment on lines +231 to +233
let output = command
.output()
.expect(format!("Failed to run {tool}").as_str());
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The expect message is constructed dynamically using format! but then passed as a static string reference. This should use expect with a closure or change to unwrap_or_else to avoid unnecessary allocations.

Copilot uses AI. Check for mistakes.
Comment on lines +310 to +317
let cpp = sources
.iter()
.find(|source| {
PathBuf::from(source)
.extension()
.map_or(false, |ext| ext == "cpp" || ext == "cc")
})
.is_some();
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Unnecessary use of is_some() after find(). The find() method can be directly chained with is_some() more idiomatically using any().

Copilot uses AI. Check for mistakes.
Comment on lines 427 to 428
.env("WASIXCC_SYSROOT", "/home/lennart/.wasix-clang/wasix-sysroot")
.env("WASIXCC_COMPILER_FLAGS", "-fPIC:-fwasm-exceptions:-Wl,-L/home/lennart/.wasix-clang/wasix-sysroot/usr/local/lib/wasm32-wasi:-I/home/lennart/.wasix-clang/wasix-sysroot/usr/local/include:-Wl,-mllvm,--wasm-enable-eh:-Wl,-mllvm,--wasm-enable-sjlj:-Wl,-mllvm,--wasm-use-legacy-eh=false:-Wl,-mllvm,--exception-model=wasm")
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Hardcoded absolute path to user home directory. This path '/home/lennart/.wasix-clang/wasix-sysroot' is specific to one developer's machine and will fail for other users. This should be made configurable through environment variables or removed if not needed.

Copilot uses AI. Check for mistakes.
test_file
.split('/')
.next_back()
.expect("The test file name can not be empty")
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

Error message uses 'can not' which should be 'cannot' (one word) in standard English.

Copilot uses AI. Check for mistakes.
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