From ae650d0c4e966c5a7df072dc2f1fb91aa96deec7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 11 May 2024 16:41:14 -0700 Subject: [PATCH 1/3] Move wast tests to their own test suite This commit moves testing of `*.wast` files out of the `all` test suite binary and into its own separate binary. The motivation for this is well-described in #4861 with one of the chief reasons being that if the test suite is run and then a new file is added re-running the test suite won't see the file. The `libtest-mimic` crate provides an easy way of regaining most of the features of the `libtest` harness such as parallel test execution and filters, meaning that it's pretty easy to switch everything over. The only slightly-tricky bit was redoing the filter for whether a test is ignored or not, but most of the pieces were copied over from the previous `build.rs` logic. Closes #4861 --- Cargo.toml | 4 + build.rs | 315 +--------------------------------------- tests/{all => }/wast.rs | 188 +++++++++++++++++++++++- 3 files changed, 187 insertions(+), 320 deletions(-) rename tests/{all => }/wast.rs (57%) diff --git a/Cargo.toml b/Cargo.toml index e20e9770c61..80e3623072c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -411,6 +411,10 @@ harness = false name = "disas" harness = false +[[test]] +name = "wast" +harness = false + [[example]] name = "tokio" required-features = ["wasi-common/tokio"] diff --git a/build.rs b/build.rs index 192abfd1192..2057d27f34c 100644 --- a/build.rs +++ b/build.rs @@ -1,322 +1,11 @@ -//! Build program to generate a program which runs all the testsuites. -//! -//! By generating a separate `#[test]` test for each file, we allow cargo test -//! to automatically run the files in parallel. - -use anyhow::Context; use std::env; -use std::fmt::Write; -use std::fs; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; -fn main() -> anyhow::Result<()> { +fn main() { println!("cargo:rerun-if-changed=build.rs"); set_commit_info_for_rustc(); - - let out_dir = PathBuf::from( - env::var_os("OUT_DIR").expect("The OUT_DIR environment variable must be set"), - ); - let mut out = String::new(); - - for strategy in &["Cranelift", "Winch"] { - writeln!(out, "#[cfg(test)]")?; - writeln!(out, "#[allow(non_snake_case)]")?; - if *strategy == "Winch" { - // We only test Winch on x86_64, for now. - writeln!(out, "{}", "#[cfg(all(target_arch = \"x86_64\"))]")?; - } - writeln!(out, "mod {} {{", strategy)?; - - with_test_module(&mut out, "misc", |out| { - test_directory(out, "tests/misc_testsuite", strategy)?; - test_directory_module(out, "tests/misc_testsuite/multi-memory", strategy)?; - test_directory_module(out, "tests/misc_testsuite/simd", strategy)?; - test_directory_module(out, "tests/misc_testsuite/tail-call", strategy)?; - test_directory_module(out, "tests/misc_testsuite/threads", strategy)?; - test_directory_module(out, "tests/misc_testsuite/memory64", strategy)?; - test_directory_module(out, "tests/misc_testsuite/component-model", strategy)?; - test_directory_module(out, "tests/misc_testsuite/function-references", strategy)?; - test_directory_module(out, "tests/misc_testsuite/gc", strategy)?; - // The testsuite of Winch is a subset of the official - // WebAssembly test suite, until parity is reached. This - // check is in place to prevent Cranelift from duplicating - // tests. - if *strategy == "Winch" { - test_directory_module(out, "tests/misc_testsuite/winch", strategy)?; - } - Ok(()) - })?; - - with_test_module(&mut out, "spec", |out| { - let spec_tests = test_directory(out, "tests/spec_testsuite", strategy)?; - // Skip running spec_testsuite tests if the submodule isn't checked - // out. - if spec_tests > 0 { - test_directory_module(out, "tests/spec_testsuite/proposals/memory64", strategy)?; - test_directory_module( - out, - "tests/spec_testsuite/proposals/function-references", - strategy, - )?; - test_directory_module(out, "tests/spec_testsuite/proposals/gc", strategy)?; - test_directory_module( - out, - "tests/spec_testsuite/proposals/multi-memory", - strategy, - )?; - test_directory_module(out, "tests/spec_testsuite/proposals/threads", strategy)?; - test_directory_module( - out, - "tests/spec_testsuite/proposals/relaxed-simd", - strategy, - )?; - test_directory_module(out, "tests/spec_testsuite/proposals/tail-call", strategy)?; - } else { - println!( - "cargo:warning=The spec testsuite is disabled. To enable, run `git submodule \ - update --remote`." - ); - } - Ok(()) - })?; - - writeln!(out, "}}")?; - } - - // Write out our auto-generated tests and opportunistically format them with - // `rustfmt` if it's installed. - let output = out_dir.join("wast_testsuite_tests.rs"); - fs::write(&output, out)?; - drop(Command::new("rustfmt").arg(&output).status()); - Ok(()) -} - -fn test_directory_module( - out: &mut String, - path: impl AsRef, - strategy: &str, -) -> anyhow::Result { - let path = path.as_ref(); - let testsuite = &extract_name(path); - with_test_module(out, testsuite, |out| test_directory(out, path, strategy)) -} - -fn test_directory( - out: &mut String, - path: impl AsRef, - strategy: &str, -) -> anyhow::Result { - let path = path.as_ref(); - let mut dir_entries: Vec<_> = path - .read_dir() - .context(format!("failed to read {:?}", path))? - .map(|r| r.expect("reading testsuite directory entry")) - .filter_map(|dir_entry| { - let p = dir_entry.path(); - let ext = p.extension()?; - // Only look at wast files. - if ext != "wast" { - return None; - } - // Ignore files starting with `.`, which could be editor temporary files - if p.file_stem()?.to_str()?.starts_with('.') { - return None; - } - Some(p) - }) - .collect(); - - dir_entries.sort(); - - let testsuite = &extract_name(path); - for entry in dir_entries.iter() { - write_testsuite_tests(out, entry, testsuite, strategy, false)?; - write_testsuite_tests(out, entry, testsuite, strategy, true)?; - } - - Ok(dir_entries.len()) -} - -/// Extract a valid Rust identifier from the stem of a path. -fn extract_name(path: impl AsRef) -> String { - path.as_ref() - .file_stem() - .expect("filename should have a stem") - .to_str() - .expect("filename should be representable as a string") - .replace(['-', '/'], "_") -} - -fn with_test_module( - out: &mut String, - testsuite: &str, - f: impl FnOnce(&mut String) -> anyhow::Result, -) -> anyhow::Result { - out.push_str("mod "); - out.push_str(testsuite); - out.push_str(" {\n"); - - let result = f(out)?; - - out.push_str("}\n"); - Ok(result) -} - -fn write_testsuite_tests( - out: &mut String, - path: impl AsRef, - testsuite: &str, - strategy: &str, - pooling: bool, -) -> anyhow::Result<()> { - let path = path.as_ref(); - let testname = extract_name(path); - - writeln!(out, "#[test]")?; - // Ignore when using QEMU for running tests (limited memory). - if ignore(testsuite, &testname, strategy) { - writeln!(out, "#[ignore]")?; - } else { - writeln!(out, "#[cfg_attr(miri, ignore)]")?; - } - - writeln!( - out, - "fn r#{}{}() {{", - &testname, - if pooling { "_pooling" } else { "" } - )?; - writeln!(out, " let _ = env_logger::try_init();")?; - writeln!( - out, - " crate::wast::run_wast(r#\"{}\"#, crate::wast::Strategy::{}, {}).unwrap();", - path.display(), - strategy, - pooling, - )?; - writeln!(out, "}}")?; - writeln!(out)?; - Ok(()) -} - -/// Ignore tests that aren't supported yet. -fn ignore(testsuite: &str, testname: &str, strategy: &str) -> bool { - assert!(strategy == "Cranelift" || strategy == "Winch"); - - // Ignore some tests for when testing Winch. - if strategy == "Winch" { - if testsuite == "misc_testsuite" { - let denylist = [ - "externref_id_function", - "int_to_float_splat", - "issue6562", - "many_table_gets_lead_to_gc", - "mutable_externref_globals", - "no_mixup_stack_maps", - "no_panic", - "simple_ref_is_null", - "table_grow_with_funcref", - ]; - return denylist.contains(&testname); - } - if testsuite == "spec_testsuite" { - let denylist = [ - "br_table", - "global", - "table_fill", - "table_get", - "table_set", - "table_grow", - "table_size", - "elem", - "select", - "unreached_invalid", - "linking", - ] - .contains(&testname); - - let ref_types = testname.starts_with("ref_"); - let simd = testname.starts_with("simd_"); - - return denylist || ref_types || simd; - } - - if testsuite == "memory64" { - return testname.starts_with("simd") || testname.starts_with("threads"); - } - - if testsuite != "winch" { - return true; - } - } - - // This is an empty file right now which the `wast` crate doesn't parse - if testname.contains("memory_copy1") { - return true; - } - - if testsuite == "gc" { - if [ - "array_copy", - "array_fill", - "array_init_data", - "array_init_elem", - "array", - "binary_gc", - "binary", - "br_on_cast_fail", - "br_on_cast", - "br_on_non_null", - "br_on_null", - "br_table", - "call_ref", - "data", - "elem", - "extern", - "func", - "global", - "if", - "linking", - "local_get", - "local_init", - "ref_as_non_null", - "ref_cast", - "ref_eq", - "ref_is_null", - "ref_null", - "ref_test", - "ref", - "return_call_indirect", - "return_call_ref", - "return_call", - "select", - "struct", - "table_sub", - "table", - "type_canon", - "type_equivalence", - "type_rec", - "type_subtyping", - "unreached_invalid", - "unreached_valid", - ] - .contains(&testname) - { - return true; - } - } - - match env::var("CARGO_CFG_TARGET_ARCH").unwrap().as_str() { - "s390x" => { - // TODO(#6530): These tests require tail calls, but s390x - // doesn't support them yet. - testsuite == "function_references" || testsuite == "tail_call" - } - - _ => false, - } } fn set_commit_info_for_rustc() { diff --git a/tests/all/wast.rs b/tests/wast.rs similarity index 57% rename from tests/all/wast.rs rename to tests/wast.rs index 89ce349f0b5..3a20e546693 100644 --- a/tests/all/wast.rs +++ b/tests/wast.rs @@ -1,5 +1,6 @@ use anyhow::Context; use bstr::ByteSlice; +use libtest_mimic::{Arguments, FormatSetting, Trial}; use once_cell::sync::Lazy; use std::path::Path; use std::sync::{Condvar, Mutex}; @@ -10,15 +11,188 @@ use wasmtime::{ use wasmtime_environ::WASM_PAGE_SIZE; use wasmtime_wast::{SpectestConfig, WastContext}; -include!(concat!(env!("OUT_DIR"), "/wast_testsuite_tests.rs")); +fn main() { + env_logger::init(); + + let mut trials = Vec::new(); + add_tests(&mut trials, "tests/spec_testsuite".as_ref()); + add_tests(&mut trials, "tests/misc_testsuite".as_ref()); + + // There's a lot of tests so print only a `.` to keep the output a + // bit more terse by default. + let mut args = Arguments::from_args(); + if args.format.is_none() { + args.format = Some(FormatSetting::Terse); + } + libtest_mimic::run(&args, trials).exit() +} + +fn add_tests(trials: &mut Vec, path: &Path) { + for entry in path.read_dir().unwrap() { + let entry = entry.unwrap(); + let path = entry.path(); + if entry.file_type().unwrap().is_dir() { + add_tests(trials, &path); + continue; + } + + if path.extension().and_then(|s| s.to_str()) != Some("wast") { + continue; + } + + for strategy in [Strategy::Cranelift, Strategy::Winch] { + for pooling in [true, false] { + let trial = Trial::test( + format!( + "{strategy:?}/{}{}", + if pooling { "pooling/" } else { "" }, + path.to_str().unwrap() + ), + { + let path = path.clone(); + move || { + run_wast(&path, strategy, pooling).map_err(|e| format!("{e:?}").into()) + } + }, + ); + trials.push(trial.with_ignored_flag(ignore(&path, strategy))); + } + } + } +} + +fn ignore(test: &Path, strategy: Strategy) -> bool { + if cfg!(miri) { + return true; + } + + // Winch only supports x86_64 at this time. + if strategy == Strategy::Winch && !cfg!(target_arch = "x86_64") { + return true; + } + + for part in test.iter() { + // Not implemented in Wasmtime yet + if part == "exception-handling" { + return true; + } + // Not implemented in Wasmtime yet + if part == "extended-const" { + return true; + } + + // TODO(#6530): These tests require tail calls, but s390x doesn't + // support them yet. + if cfg!(target_arch = "s390x") { + if part == "function-references" || part == "tail-call" { + return true; + } + } + + // Disable spec tests for proposals that Winch does not implement yet. + if strategy == Strategy::Winch { + let part = part.to_str().unwrap(); + if false + // wasm proposals that Winch doesn't support + || part == "references" + || part == "tail-call" + || part == "gc" + || part == "threads" + || part == "multi-memory" + // || part == "simd" + || part == "relaxed-simd" + || part == "function-references" + // tests in misc_testsuite that Winch doesn't support + || part == "no-panic.wast" + || part == "externref-id-function.wast" + || part == "int-to-float-splat.wast" + || part == "issue6562.wast" + || part == "many_table_gets_lead_to_gc.wast" + || part == "mutable_externref_globals.wast" + || part == "no-mixup-stack-maps.wast" + || part == "simple_ref_is_null.wast" + || part == "table_grow_with_funcref.wast" + // Tests in the spec test suite Winch doesn't support + || part.starts_with("simd") + || part.starts_with("ref_") + || part == "threads.wast" + || part == "br_table.wast" + || part == "global.wast" + || part == "table_fill.wast" + || part == "table_get.wast" + || part == "table_set.wast" + || part == "table_grow.wast" + || part == "table_size.wast" + || part == "elem.wast" + || part == "select.wast" + || part == "unreached-invalid.wast" + || part == "linking.wast" + { + return true; + } + } + + // Implementation of the GC proposal is a work-in-progress, this is + // a list of all currently known-to-fail tests. + if part == "gc" { + return [ + "array_copy.wast", + "array_fill.wast", + "array_init_data.wast", + "array_init_elem.wast", + "array.wast", + "binary_gc.wast", + "binary.wast", + "br_on_cast_fail.wast", + "br_on_cast.wast", + "br_on_non_null.wast", + "br_on_null.wast", + "br_table.wast", + "call_ref.wast", + "data.wast", + "elem.wast", + "extern.wast", + "func.wast", + "global.wast", + "if.wast", + "linking.wast", + "local_get.wast", + "local_init.wast", + "ref_as_non_null.wast", + "ref_cast.wast", + "ref_eq.wast", + "ref_is_null.wast", + "ref_null.wast", + "ref_test.wast", + "ref.wast", + "return_call_indirect.wast", + "return_call_ref.wast", + "return_call.wast", + "select.wast", + "struct.wast", + "table_sub.wast", + "table.wast", + "type_canon.wast", + "type_equivalence.wast", + "type-rec.wast", + "type-subtyping.wast", + "unreached-invalid.wast", + "unreached_valid.wast", + ] + .iter() + .any(|i| test.ends_with(i)); + } + } + + false +} // Each of the tests included from `wast_testsuite_tests` will call this // function which actually executes the `wast` test suite given the `strategy` // to compile it. -fn run_wast(wast: &str, strategy: Strategy, pooling: bool) -> anyhow::Result<()> { - drop(env_logger::try_init()); - - let wast_bytes = std::fs::read(wast).with_context(|| format!("failed to read `{}`", wast))?; +fn run_wast(wast: &Path, strategy: Strategy, pooling: bool) -> anyhow::Result<()> { + let wast_bytes = + std::fs::read(wast).with_context(|| format!("failed to read `{}`", wast.display()))?; let wast = Path::new(wast); @@ -35,7 +209,7 @@ fn run_wast(wast: &str, strategy: Strategy, pooling: bool) -> anyhow::Result<()> || feature_found_src(&wast_bytes, "shared)"); if pooling && use_shared_memory { - eprintln!("skipping pooling test with shared memory"); + log::warn!("skipping pooling test with shared memory"); return Ok(()); } @@ -167,7 +341,7 @@ fn run_wast(wast: &str, strategy: Strategy, pooling: bool) -> anyhow::Result<()> let mut wast_context = WastContext::new(store); wast_context.register_spectest(&SpectestConfig { use_shared_memory, - suppress_prints: false, + suppress_prints: true, })?; wast_context .run_buffer(wast.to_str().unwrap(), &wast_bytes) From c5f981b7a6db4aa3459d51dee585ab430b782db4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 11 May 2024 17:41:32 -0700 Subject: [PATCH 2/3] Fix the `all` suite --- tests/all/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/all/main.rs b/tests/all/main.rs index fcab06d3503..d2e5a105bfd 100644 --- a/tests/all/main.rs +++ b/tests/all/main.rs @@ -41,7 +41,6 @@ mod traps; mod types; mod wait_notify; mod wasi_testsuite; -mod wast; // Currently Winch is only supported in x86_64. #[cfg(all(target_arch = "x86_64"))] mod winch; From 5aedde10377261ab6b2dd3002cd24f62b8023f7f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 13 May 2024 08:41:50 -0700 Subject: [PATCH 3/3] Review comments --- tests/wast.rs | 77 +++++++++++++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 40 deletions(-) diff --git a/tests/wast.rs b/tests/wast.rs index 3a20e546693..e5751f330ff 100644 --- a/tests/wast.rs +++ b/tests/wast.rs @@ -15,8 +15,10 @@ fn main() { env_logger::init(); let mut trials = Vec::new(); - add_tests(&mut trials, "tests/spec_testsuite".as_ref()); - add_tests(&mut trials, "tests/misc_testsuite".as_ref()); + if !cfg!(miri) { + add_tests(&mut trials, "tests/spec_testsuite".as_ref()); + add_tests(&mut trials, "tests/misc_testsuite".as_ref()); + } // There's a lot of tests so print only a `.` to keep the output a // bit more terse by default. @@ -62,10 +64,6 @@ fn add_tests(trials: &mut Vec, path: &Path) { } fn ignore(test: &Path, strategy: Strategy) -> bool { - if cfg!(miri) { - return true; - } - // Winch only supports x86_64 at this time. if strategy == Strategy::Winch && !cfg!(target_arch = "x86_64") { return true; @@ -92,42 +90,41 @@ fn ignore(test: &Path, strategy: Strategy) -> bool { // Disable spec tests for proposals that Winch does not implement yet. if strategy == Strategy::Winch { let part = part.to_str().unwrap(); - if false - // wasm proposals that Winch doesn't support - || part == "references" - || part == "tail-call" - || part == "gc" - || part == "threads" - || part == "multi-memory" - // || part == "simd" - || part == "relaxed-simd" - || part == "function-references" + let unsupported = [ + // wasm proposals that Winch doesn't support, + "references", + "tail-call", + "gc", + "threads", + "multi-memory", + "relaxed-simd", + "function-references", // tests in misc_testsuite that Winch doesn't support - || part == "no-panic.wast" - || part == "externref-id-function.wast" - || part == "int-to-float-splat.wast" - || part == "issue6562.wast" - || part == "many_table_gets_lead_to_gc.wast" - || part == "mutable_externref_globals.wast" - || part == "no-mixup-stack-maps.wast" - || part == "simple_ref_is_null.wast" - || part == "table_grow_with_funcref.wast" + "no-panic.wast", + "externref-id-function.wast", + "int-to-float-splat.wast", + "issue6562.wast", + "many_table_gets_lead_to_gc.wast", + "mutable_externref_globals.wast", + "no-mixup-stack-maps.wast", + "simple_ref_is_null.wast", + "table_grow_with_funcref.wast", // Tests in the spec test suite Winch doesn't support - || part.starts_with("simd") - || part.starts_with("ref_") - || part == "threads.wast" - || part == "br_table.wast" - || part == "global.wast" - || part == "table_fill.wast" - || part == "table_get.wast" - || part == "table_set.wast" - || part == "table_grow.wast" - || part == "table_size.wast" - || part == "elem.wast" - || part == "select.wast" - || part == "unreached-invalid.wast" - || part == "linking.wast" - { + "threads.wast", + "br_table.wast", + "global.wast", + "table_fill.wast", + "table_get.wast", + "table_set.wast", + "table_grow.wast", + "table_size.wast", + "elem.wast", + "select.wast", + "unreached-invalid.wast", + "linking.wast", + ]; + + if unsupported.contains(&part) || part.starts_with("simd") || part.starts_with("ref_") { return true; } }