Merge #563

563: improve wasitest infra and fix fseek test r=MarkMcCaskey a=MarkMcCaskey

This PR:
- fixes arg parsing from comments
- fixes the mapdir test to have the native code doing the same thing
- makes `wasitests-generate` output stdout/stderr by default
- adds function to print stdout and stderr for a command if it fails
- compiles wasm with size optimizations
- strips generated wasm with `wasm-strip`

This makes wasm module sizes go down from 1.7, 1.9Mb to ~60kb, 80kb.  Preventing git bloat and reducing likely bottleneck of reading wasm modules from disk

Co-authored-by: Mark McCaskey <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>

CHANGELOG.md

@@ -5,6 +5,10 @@ All PRs to the Wasmer repository must add to this file.
 Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
+- [#563](https://github.com/wasmerio/wasmer/pull/563) Improve wasi testing infrastructure
+  - fixes arg parsing from comments & fixes the mapdir test to have the native code doing the same thing as the WASI code
+  - makes wasitests-generate output stdout/stderr by default & adds function to print stdout and stderr for a command if it fails
+  - compiles wasm with size optimizations & strips generated wasm with wasm-strip
 - [#554](https://github.com/wasmerio/wasmer/pull/554) Finish implementation of `wasi::fd_seek`, fix bug in filestat
 
 ## 0.5.5

Makefile

@@ -8,7 +8,7 @@ generate-emtests:
 	WASM_EMSCRIPTEN_GENERATE_EMTESTS=1 cargo build -p wasmer-emscripten-tests --release
 
 generate-wasitests:
-	WASM_WASI_GENERATE_WASITESTS=1 cargo build -p wasmer-wasi-tests --release
+	WASM_WASI_GENERATE_WASITESTS=1 cargo build -p wasmer-wasi-tests --release -vv
 
 generate: generate-spectests generate-emtests generate-wasitests
 

lib/wasi-tests/build/wasitests.rs

@@ -20,7 +20,6 @@ static BANNER: &str = "// !!! THIS IS A GENERATED FILE !!!
 // Files autogenerated with cargo build (build/wasitests.rs).\n";
 
 pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
-    dbg!(file);
     let mut output_path = PathBuf::from(file);
     output_path.set_extension("out");
 
@@ -31,12 +30,14 @@ pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
         nn
     };
 
-    Command::new("rustc")
+    println!("Compiling program {} to native", file);
+    let native_out = Command::new("rustc")
         .arg(file)
         .arg("-o")
         .arg(&normalized_name)
         .output()
         .expect("Failed to compile program to native code");
+    print_info_on_error(&native_out, "COMPILATION FAILED");
 
     #[cfg(unix)]
     {
@@ -57,18 +58,29 @@ pub fn compile(file: &str, ignores: &HashSet<String>) -> Option<String> {
     let result = Command::new(&normalized_name)
         .output()
         .expect("Failed to execute native program");
+    print_info_on_error(&result, "NATIVE PROGRAM FAILED");
 
     std::fs::remove_file(&normalized_name).expect("could not delete executable");
     let wasm_out_name = format!("{}.wasm", &normalized_name);
 
-    Command::new("rustc")
+    let wasm_compilation_out = Command::new("rustc")
         .arg("+nightly")
         .arg("--target=wasm32-wasi")
+        .arg("-C")
+        .arg("opt-level=s")
         .arg(file)
         .arg("-o")
         .arg(&wasm_out_name)
         .output()
         .expect("Failed to compile program to native code");
+    print_info_on_error(&wasm_compilation_out, "WASM COMPILATION");
+
+    // to prevent commiting huge binary blobs forever
+    let wasm_strip_out = Command::new("wasm-strip")
+        .arg(&wasm_out_name)
+        .output()
+        .expect("Failed to strip compiled wasm module");
+    print_info_on_error(&wasm_strip_out, "STRIPPING WASM");
 
     let ignored = if ignores.contains(&rs_module_name) {
         "\n#[ignore]"
@@ -193,25 +205,36 @@ fn extract_args_from_source_file(source_code: &str) -> Option<Args> {
         {
             let tokenized = arg_line
                 .split_whitespace()
+                // skip trailing space
                 .skip(1)
                 .map(String::from)
                 .collect::<Vec<String>>();
-            match tokenized[1].as_ref() {
+            let command_name = {
+                let mut cn = tokenized[0].clone();
+                assert_eq!(
+                    cn.pop(),
+                    Some(':'),
+                    "Final character of argname must be a colon"
+                );
+                cn
+            };
+
+            match command_name.as_ref() {
                 "mapdir" => {
-                    if let [alias, real_dir] = &tokenized[2].split(':').collect::<Vec<&str>>()[..] {
+                    if let [alias, real_dir] = &tokenized[1].split(':').collect::<Vec<&str>>()[..] {
                         args.mapdir.push((alias.to_string(), real_dir.to_string()));
                     } else {
                         eprintln!(
                             "Parse error in mapdir {} not parsed correctly",
-                            &tokenized[2]
+                            &tokenized[1]
                         );
                     }
                 }
                 "env" => {
-                    if let [name, val] = &tokenized[2].split('=').collect::<Vec<&str>>()[..] {
+                    if let [name, val] = &tokenized[1].split('=').collect::<Vec<&str>>()[..] {
                         args.envvars.push((name.to_string(), val.to_string()));
                     } else {
-                        eprintln!("Parse error in env {} not parsed correctly", &tokenized[2]);
+                        eprintln!("Parse error in env {} not parsed correctly", &tokenized[1]);
                     }
                 }
                 e => {
@@ -223,3 +246,17 @@ fn extract_args_from_source_file(source_code: &str) -> Option<Args> {
     }
     None
 }
+
+fn print_info_on_error(output: &std::process::Output, context: &str) {
+    if !output.status.success() {
+        println!("{}", context);
+        println!(
+            "stdout:\n{}",
+            std::str::from_utf8(&output.stdout[..]).unwrap()
+        );
+        eprintln!(
+            "stderr:\n{}",
+            std::str::from_utf8(&output.stderr[..]).unwrap()
+        );
+    }
+}

lib/wasi-tests/tests/README.md

@@ -0,0 +1,5 @@
+Most of the files here are generated.
+
+`_common.rs` is a file containing a macro that the generated tests use to avoid code duplication.
+
+If you want to add new features, edit `_common.rs` and `wasi-tests/build/wasitests.rs` to use the changed macro.

lib/wasi-tests/tests/wasitests/envvar.rs

@@ -4,7 +4,7 @@ fn test_envvar() {
         "../../wasitests/envvar.wasm",
         "envvar",
         vec![],
-        vec![],
+        vec!["DOG=1".to_string(), "CAT=2".to_string(),],
         "../../wasitests/envvar.out"
     );
 }

lib/wasi-tests/wasitests/mapdir.rs

@@ -4,9 +4,11 @@
 use std::fs;
 
 fn main() {
-    // #[cfg(not(target_os = "wasi"))]
-    // let read_dir = fs::read_dir("wasitests/test_fs/hamlet").unwrap();
-    // #[cfg(target_os = "wasi")]
+    #[cfg(not(target_os = "wasi"))]
+    let cur_dir = std::env::current_dir().unwrap();
+    #[cfg(not(target_os = "wasi"))]
+    std::env::set_current_dir("wasitests/test_fs/hamlet").unwrap();
+
     let read_dir = fs::read_dir(".").unwrap();
     let mut out = vec![];
     for entry in read_dir {
@@ -17,4 +19,7 @@ fn main() {
     for p in out {
         println!("{}", p);
     }
+    // return to the current directory
+    #[cfg(not(target_os = "wasi"))]
+    std::env::set_current_dir(cur_dir).unwrap();
 }

rustfmt.toml

@@ -1,4 +1,5 @@
 ignore = [
     "src/spectests",
-    "src/emtests",
+    "src/emscripten-tests",
+    "src/wasi-tests",
 ]