Address requested changes

Author: Ari Breitkreuz

crates/cli/src/bin/wasm-bindgen-test-runner/main.rs

@@ -256,11 +256,10 @@ fn coverage_args(tmpdir: &Path) -> Option<PathBuf> {
     }
 
     // Profraw path is ignored if coverage isn't enabled
-    env::var_os("WASM_BINDGEN_TEST_COVERAGE")?;
-    log::warn!("Coverage support is still considered highly experimental!");
+    env::var_os("WASM_BINDGEN_UNSTABLE_TEST_COVERAGE")?;
     // TODO coverage link to wasm-bindgen book documenting correct usage
 
-    match env::var_os("WASM_BINDGEN_TEST_PROFRAW_OUT") {
+    match env::var_os("WASM_BINDGEN_TEST_UNSTABLE_PROFRAW_OUT") {
         Some(s) => {
             let mut buf = PathBuf::from(s);
             if buf.is_dir() {

crates/cli/src/bin/wasm-bindgen-test-runner/server.rs

@@ -245,24 +245,21 @@ pub fn spawn(
             };
             return set_isolate_origin_headers(Response::from_data("text/html", s));
         } else if request.url() == "/__coverage/dump" {
-            let profraw_path = coverage.as_ref().expect(
-                "Received coverage dump request but server wasn't set up to accept coverage",
-            );
-            // This is run after all tests are done and dumps the data received in the request
-            // into a single profraw file
-            let mut profraw =
-                std::fs::File::create(profraw_path).expect("Couldn't create .profraw for coverage");
-            let mut data = Vec::new();
-            request
-                .data()
-                .expect("Expected coverage data in body")
-                .read_to_end(&mut data)
-                .expect("Failed to read message body");
-
-            profraw
-                .write_all(&data)
-                .expect("Couldn't dump coverage data to profraw");
-            return Response::text("Coverage dumped");
+            fn internal_err(s: &str) -> Response {
+                log::error!("{s}");
+                let mut ret = Response::text(s);
+                ret.status_code = 500;
+                ret
+            }
+            let Some(profraw_path) = &coverage else {
+                return internal_err(
+                    "Received coverage dump request but server wasn't set up to accept coverage",
+                );
+            };
+            return match handle_coverage_dump(profraw_path, request) {
+                Ok(()) => Response::empty_204(),
+                Err(e) => internal_err(&format!("Failed to dump coverage: {e}")),
+            };
         }
 
         // Otherwise we need to find the asset here. It may either be in our
@@ -311,6 +308,21 @@ pub fn spawn(
     }
 }
 
+fn handle_coverage_dump(profraw_path: &Path, request: &Request) -> anyhow::Result<()> {
+    // This is run after all tests are done and dumps the data received in the request
+    // into a single profraw file
+    let mut profraw = std::fs::File::create(profraw_path)?;
+    let mut data = Vec::new();
+    if let Some(mut r_data) = request.data() {
+        r_data.read_to_end(&mut data)?;
+    }
+    // Warnings about empty data should have already been handled by
+    // the client
+
+    profraw.write_all(&data)?;
+    Ok(())
+}
+
 /*
  * Set the Cross-Origin-Opener-Policy and Cross-Origin_Embedder-Policy headers
  * on the Server response to enable worker context sharing, as described in:

crates/test/Cargo.toml

@@ -10,9 +10,7 @@ rust-version = "1.57"
 
 [features]
 default = []
-experimental = ["coverage"]
-
-coverage = ["minicov"]
+unstable-coverage = ["minicov"]
 
 [dependencies]
 console_error_panic_hook = '0.1'

crates/test/src/coverage.rs

@@ -1,6 +1,6 @@
 use wasm_bindgen::prelude::wasm_bindgen;
 
-#[cfg(feature = "coverage")]
+#[cfg(feature = "unstable-coverage")]
 #[wasm_bindgen]
 pub fn __wbgtest_cov_dump() -> Vec<u8> {
     let mut coverage = Vec::new();
@@ -16,10 +16,10 @@ pub fn __wbgtest_cov_dump() -> Vec<u8> {
     coverage
 }
 
-/// Called when setting WASM_BINDGEN_TEST_COVERAGE but coverage feature is disabled.
+/// Called when setting WASM_BINDGEN_UNSTABLE_TEST_COVERAGE but coverage feature is disabled.
 /// Currently not being used because of issues in the interpreter regarding profiling
 /// information which cause an error before we get here.
-#[cfg(not(feature = "coverage"))]
+#[cfg(not(feature = "unstable-coverage"))]
 #[wasm_bindgen]
 pub fn __wbgtest_cov_dump() -> Vec<u8> {
     console_error!(

crates/test/src/lib.rs

@@ -21,15 +21,6 @@ macro_rules! console_log {
     )
 }
 
-/// Helper macro which acts like `println!` only routes to `console.warn`
-/// instead.
-#[macro_export]
-macro_rules! console_warn {
-    ($($arg:tt)*) => (
-        $crate::__rt::log_warn(&format_args!($($arg)*))
-    )
-}
-
 /// Helper macro which acts like `println!` only routes to `console.error`
 /// instead.
 #[macro_export]

crates/test/src/rt/mod.rs

@@ -237,10 +237,6 @@ extern "C" {
     #[doc(hidden)]
     pub fn js_console_error(s: &str);
 
-    #[wasm_bindgen(js_namespace = console, js_name = warn)]
-    #[doc(hidden)]
-    pub fn js_console_warn(s: &str);
-
     // General-purpose conversion into a `String`.
     #[wasm_bindgen(js_name = String)]
     fn stringify(val: &JsValue) -> String;
@@ -251,11 +247,6 @@ pub fn log(args: &fmt::Arguments) {
     js_console_log(&args.to_string());
 }
 
-/// Internal implementation detail of the `console_warn!` macro.
-pub fn log_warn(args: &fmt::Arguments) {
-    js_console_warn(&args.to_string());
-}
-
 /// Internal implementation detail of the `console_error!` macro.
 pub fn log_error(args: &fmt::Arguments) {
     js_console_error(&args.to_string());

crates/wasm-interpreter/src/lib.rs

@@ -74,7 +74,7 @@ impl Interpreter {
     /// the `module` passed to `interpret` below.
     pub fn new(module: &Module) -> Result<Interpreter, anyhow::Error> {
         let mut ret = Interpreter::default();
-        ret.coverage = std::env::var_os("WASM_BINDGEN_TEST_COVERAGE").is_some();
+        ret.coverage = std::env::var_os("WASM_BINDGEN_UNSTABLE_TEST_COVERAGE").is_some();
 
         // The descriptor functions shouldn't really use all that much memory
         // (the LLVM call stack, now the wasm stack). To handle that let's give
@@ -227,7 +227,7 @@ impl Interpreter {
         log::debug!("arguments {:?}", args);
         let local = match &func.kind {
             walrus::FunctionKind::Local(l) => l,
-            _ => panic!("can only call locally defined functions. If you're compiling with profiling information you might want to set WASM_BINDGEN_TEST_COVERAGE (--coverage flag in wasm-pack)"),
+            _ => panic!("can only call locally defined functions. If you're compiling with profiling information you might want to set WASM_BINDGEN_UNSTABLE_TEST_COVERAGE (--coverage flag in wasm-pack)"),
         };
 
         let entry = local.entry_block();
@@ -273,7 +273,7 @@ impl Frame<'_> {
             Instr::Const(c) => match c.value {
                 Value::I32(n) => stack.push(n),
                 Value::I64(_) if coverage => stack.push(0),
-                _ => panic!("non-i32 constant. If you're compiling with profiling information you might want to set WASM_BINDGEN_TEST_COVERAGE (--coverage flag in wasm-pack)"),
+                _ => panic!("non-i32 constant. If you're compiling with profiling information you might want to set WASM_BINDGEN_UNSTABLE_TEST_COVERAGE (--coverage flag in wasm-pack)"),
             },
             Instr::LocalGet(e) => stack.push(self.locals.get(&e.local).cloned().unwrap_or(0)),
             Instr::LocalSet(e) => {