Add some dox

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

@@ -16,6 +16,11 @@ use shell::Shell;
 
 /// Execute a headless browser tests against a server running on `server`
 /// address.
+///
+/// This function will take care of everything from spawning the WebDriver
+/// binary, controlling it, running tests, scraping output, displaying output,
+/// etc. It will return `Ok` if all tests finish successfully, and otherwise it
+/// will return an error if some tests failed.
 pub fn run(server: &SocketAddr, shell: &Shell) -> Result<(), Error> {
     let (driver, args) = Driver::find()?;
     println!("Running headless tests in {} with `{}`",
@@ -155,6 +160,10 @@ pub fn run(server: &SocketAddr, shell: &Shell) -> Result<(), Error> {
         println!("console.log div contained:\n{}", tab(&errors));
     }
 
+    if !output.contains("test result: ok") {
+        bail!("some tests failed")
+    }
+
     Ok(())
 }
 
@@ -165,6 +174,15 @@ enum Driver {
 }
 
 impl Driver {
+    /// Attempts to find an appropriate WebDriver server binary to execute tests
+    /// with. Performs a number of heuristics to find one available, including:
+    ///
+    /// * Env vars like `GECKODRIVER` point to the path to a binary to execute.
+    /// * Otherwise, `PATH` is searched for an appropriate binary.
+    ///
+    /// In both cases a list of auxiliary arguments is also returned which is
+    /// configured through env vars like `GECKODRIVER_ARGS` to support extra
+    /// arguments to the driver's invocation.
     fn find() -> Result<(Driver, Vec<String>), Error> {
         let env_args = |name: &str| {
             env::var(format!("{}_ARGS", name.to_uppercase()))
@@ -243,6 +261,10 @@ enum Method<'a> {
     Delete,
 }
 
+// Below here is a bunch of details of the WebDriver protocol implementation.
+// I'm not too too familiar with them myself, but these seem to work! I mostly
+// copied the `webdriver-client` crate when writing the below bindings.
+
 impl Client {
     fn new_session(&self, driver: &Driver) -> Result<String, Error> {
         match driver {

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

@@ -1,3 +1,16 @@
+//! A "wrapper binary" used to execute wasm files as tests
+//!
+//! This binary is intended to be used as a "test runner" for wasm binaries,
+//! being compatible with `cargo test` for the wasm target. It will
+//! automatically execute `wasm-bindgen` (or the equivalent thereof) and then
+//! execute either Node.js over the tests or start a server which a browser can
+//! be used to run against to execute tests. In a browser mode if `CI` is in the
+//! environment then it'll also attempt headless testing, spawning the server in
+//! the background and then using the WebDriver protocol to execute tests.
+//!
+//! For more documentation about this see the `wasm-bindgen-test` crate README
+//! and source code.
+
 extern crate curl;
 extern crate env_logger;
 #[macro_use]

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

@@ -38,11 +38,6 @@ pub fn execute(module: &str, tmpdir: &Path, args: &[OsString], tests: &[String])
             const support = require("./{0}");
             const wasm = require("./{0}_bg");
 
-            // Hack for now to support 0 tests in a binary. This should be done
-            // better...
-            if (support.Context === undefined)
-                process.exit(0);
-
             cx = new support.Context();
 
             // Forward runtime arguments. These arguments are also arguments to the
@@ -75,6 +70,9 @@ pub fn execute(module: &str, tmpdir: &Path, args: &[OsString], tests: &[String])
     fs::write(&js_path, js_to_execute)
         .context("failed to write JS file")?;
 
+    // Augment `NODE_PATH` so things like `require("tests/my-custom.js")` work
+    // and Rust code can import from custom JS shims. This is a bit of a hack
+    // and should probably be removed at some point.
     let path = env::var("NODE_PATH").unwrap_or_default();
     let mut path = env::split_paths(&path).collect::<Vec<_>>();
     path.push(env::current_dir().unwrap());

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

@@ -19,10 +19,16 @@ pub fn spawn(
         import {{ Context }} from './{0}';
         import * as wasm from './{0}_bg';
 
+        // Now that we've gotten to the point where JS is executing, update our
+        // status text as at this point we should be asynchronously fetching the
+        // wasm module.
         document.getElementById('output').innerHTML = "Loading wasm module...";
 
         async function main(test) {{
+            // this is a facet of using wasm2es6js, a hack until browsers have
+            // native ESM support for wasm modules.
             await wasm.booted;
+
             const cx = Context.new();
             window.global_cx = cx;
 
@@ -51,6 +57,11 @@ pub fn spawn(
     // No browser today supports a wasm file as ES modules natively, so we need
     // to shim it. Use `wasm2es6js` here to fetch an appropriate URL and look
     // like an ES module with the wasm module under the hood.
+    //
+    // TODO: don't reparse the wasm module here, should pass the
+    //       `parity_wasm::Module struct` directly from the output of
+    //       `wasm-bindgen` previously here and avoid unnecessary
+    //       parsing.
     let wasm_name = format!("{}_bg.wasm", module);
     let wasm = fs::read(tmpdir.join(&wasm_name))?;
     let output = Config::new()
@@ -63,7 +74,10 @@ pub fn spawn(
     // For now, always run forever on this port. We may update this later!
     let tmpdir = tmpdir.to_path_buf();
     let srv = Server::new(addr, move |request| {
-        // The root path gets our canned `index.html`
+        // The root path gets our canned `index.html`. The two templates here
+        // differ slightly in the default routing of `console.log`, going to an
+        // HTML element during headless testing so we can try to scrape its
+        // output.
         if request.url() == "/" {
             let s = if headless {
                 include_str!("index-headless.html")

crates/test/src/lib.rs

@@ -3,6 +3,7 @@
 //! More documentation can be found in the README for this crate!
 
 #![feature(use_extern_macros)]
+#![deny(missing_docs)]
 
 extern crate wasm_bindgen_test_macro;
 extern crate wasm_bindgen;
@@ -46,5 +47,4 @@ macro_rules! wasm_bindgen_test_configure {
 }
 
 #[path = "rt/mod.rs"]
-#[doc(hidden)]
 pub mod __rt;

crates/test/src/rt/browser.rs

@@ -6,6 +6,10 @@
 use wasm_bindgen::prelude::*;
 use js_sys::Error;
 
+/// Implementation of `Formatter` for browsers.
+///
+/// Routes all output to a `pre` on the page currently. Eventually this probably
+/// wants to be a pretty table with colors and folding and whatnot.
 pub struct Browser {
     pre: Element,
 }
@@ -29,6 +33,8 @@ extern {
 }
 
 impl Browser {
+    /// Creates a new instance of `Browser`, assuming that its APIs will work
+    /// (requires `Node::new()` to have return `None` first).
     pub fn new() -> Browser {
         let pre = document.getElementById("output");
         pre.set_inner_html("");

crates/test/src/rt/detect.rs

@@ -1,3 +1,5 @@
+//! Runtime detection of whether we're in node.js or a browser.
+
 use wasm_bindgen::prelude::*;
 use js_sys::{Array, Function};
 

crates/test/src/rt/mod.rs

@@ -1,4 +1,91 @@
-#![doc(hidden)]
+//! Internal-only runtime module used for the `wasm_bindgen_test` crate.
+//!
+//! No API contained in this module will respect semver, these should all be
+//! considered private APIs.
+
+// # Architecture of `wasm_bindgen_test`
+//
+// This module can seem a bit funky, but it's intended to be the runtime support
+// of the `#[wasm_bindgen_test]` macro and be amenable to executing wasm test
+// suites. The general idea is that for a wasm test binary there will be a set
+// of functions tagged `#[wasm_bindgen_test]`. It's the job of the runtime
+// support to execute all of these functions, collecting and collating the
+// results.
+//
+// This runtime support works in tandem with the `wasm-bindgen-test-runner`
+// binary as part of the `wasm-bindgen-cli` package.
+//
+// ## High Level Overview
+//
+// Here's a rough and (semi) high level overview of what happens when this crate
+// runs.
+//
+// * First, the user runs `cargo test --target wasm32-unknown-unknown`
+//
+// * Cargo then compiles all the test suites (aka `tests/*.rs`) as wasm binaries
+//   (the `bin` crate type). These binaries all have entry points that are
+//   `main` functions, but it's actually not used. The binaries are also
+//   compiled with `--test`, which means they're linked to the standard `test`
+//   crate, but this crate doesn't work on wasm and so we bypass it entirely.
+//
+// * Instead of using `#[test]`, which doesn't work, users wrote tests with
+//   `#[wasm_bindgen_test]`. This macro expands to a bunch of `#[no_mangle]`
+//   functions with known names (currently named `__wbg_test_*`).
+//
+// * Next up, Cargo was configured via its test runner support to execute the
+//   `wasm-bindgen-test-runner` binary. Instead of what Cargo normally does,
+//   executing `target/wasm32-unknown-unknown/debug/deps/foo-xxxxx.wasm` (which
+//   will fail as we can't actually execute was binaries), Cargo will execute
+//   `wasm-bindgen-test-runner target/.../foo-xxxxx.wasm`.
+//
+// * The `wasm-bindgen-test-runner` binary takes over. It runs `wasm-bindgen`
+//   over the binary, generating JS bindings and such. It also figures out if
+//   we're running in node.js or a browser.
+//
+// * The `wasm-bindgen-test-runner` binary generates a JS entry point. This
+//   entry point creates a `Context` below. The runner binary also parses the
+//   wasm file and finds all functions that are named `__wbg_test_*`. The
+//   generate file gathers up all these functions into an array and then passes
+//   them to `Context` below. Note that these functions are passed as *JS
+//   values*.
+//
+// * Somehow, the runner then executes the JS file. This may be with node.js, it
+//   may serve up files in a server and wait for the user, or it serves up files
+//   in a server and starts headless testing.
+//
+// * Testing starts, it loads all the modules using either ES imports or Node
+//   `require` statements. Everything is loaded in JS now.
+//
+// * A `Context` is created. The `Context` is forwarded the CLI arguments of the
+//   original `wasm-bindgen-test-runner` in an environment specific fashion.
+//   This is used for test filters today.
+//
+// * The `Context::run` function is called. Again, the generated JS has gathered
+//   all wasm tests to be executed into a list, and it's passed in here. Again,
+//   it's very important that these functions are JS values, not function
+//   pointers in Rust.
+//
+// * Next, `Context::run` will proceed to execute all of the functions. When a
+//   function is executed we're invoking a JS function, which means we're
+//   allowed to catch exceptions. This is how we handle failing tests without
+//   aborting the entire process.
+//
+// * When a test executes, it's executing an entry point generated by
+//   `#[wasm_bindgen_test]`. The test informs the `Context` of its name and
+//   other metadata, and then `Context::execute` actually invokes the tests
+//   itself (which currently is a unit function).
+//
+// * Finally, after all tests are run, the `Context` prints out all the results.
+//
+// ## Other various notes
+//
+// Phew, that was a lot! Some other various bits and pieces you may want to be
+// aware of are throughout the code. These include things like how printing
+// results is different in node vs a browser, or how we even detect if we're in
+// node or a browser.
+//
+// Overall this is all somewhat in flux as it's pretty new, and feedback is
+// always of course welcome!
 
 use std::cell::{RefCell, Cell};
 use std::fmt;
@@ -18,14 +105,38 @@ pub mod detect;
 /// drive test execution.
 #[wasm_bindgen]
 pub struct Context {
+    /// An optional filter used to restrict which tests are actually executed
+    /// and which are ignored. This is passed via the `args` function which
+    /// comes from the command line of `wasm-bindgen-test-runner`. Currently
+    /// this is the only "CLI option"
     filter: Option<String>,
+
+    /// The current test that is executing. If `None` no test is executing, if
+    /// `Some` it's the name of the tests.
     current_test: RefCell<Option<String>>,
+
+    /// Counter of the number of tests that have succeeded.
     succeeded: Cell<usize>,
+
+    /// Counter of the number of tests that have been ignored
     ignored: Cell<usize>,
+
+    /// A list of all tests which have failed. The first element of this pair is
+    /// the name of the test that failed, and the second is all logging
+    /// information (formatted) associated with the failure.
     failures: RefCell<Vec<(String, String)>>,
+
+    /// Sink for `console.log` invocations when a test is running. This is
+    /// filled in by the `Context::console_log` function below while a test is
+    /// executing (aka while `current_test` above is `Some`).
     current_log: RefCell<String>,
     current_error: RefCell<String>,
+
+    /// Flag set as a test executes if it was actually ignored.
     ignore_this_test: Cell<bool>,
+
+    /// How to actually format output, either node.js or browser-specific
+    /// implementation.
     formatter: Box<Formatter>,
 }
 
@@ -48,6 +159,7 @@ extern {
     fn stringify(val: &JsValue) -> String;
 }
 
+/// Internal implementation detail of the `console_log!` macro.
 pub fn log(args: &fmt::Arguments) {
     console_log(&args.to_string());
 }
@@ -55,6 +167,11 @@ pub fn log(args: &fmt::Arguments) {
 #[wasm_bindgen]
 
 impl Context {
+    /// Creates a new context ready to run tests.
+    ///
+    /// A `Context` is the main structure through which test execution is
+    /// coordinated, and this will collect output and results for all executed
+    /// tests.
     #[wasm_bindgen(constructor)]
     pub fn new() -> Context {
         console_error_panic_hook::set_once();
@@ -82,6 +199,11 @@ impl Context {
     /// Eventually this will be used to support flags, but for now it's just
     /// used to support test filters.
     pub fn args(&mut self, args: Vec<JsValue>) {
+        // Here we want to reject all flags like `--foo` or `-f` as we don't
+        // support anything, and also we only support at most one non-flag
+        // argument as a test filter.
+        //
+        // Everything else is rejected.
         for arg in args {
             let arg = arg.as_string().unwrap();
             if arg.starts_with("-") {
@@ -101,6 +223,9 @@ impl Context {
     /// still catch JS exceptions.
     pub fn run(&self, tests: Vec<JsValue>) -> bool {
         let this = JsValue::null();
+
+        // Each entry point has one argument, a raw pointer to this `Context`,
+        // so build up that array we'll be passing all the functions.
         let args = Array::new();
         args.push(&JsValue::from(self as *const Context as u32));
 
@@ -110,6 +235,9 @@ impl Context {
 
         for test in tests {
             self.ignore_this_test.set(false);
+
+            // Use `Function.apply` to catch any exceptions and otherwise invoke
+            // the test.
             let test = Function::from(test);
             match test.apply(&this, &args) {
                 Ok(_) => {
@@ -192,10 +320,19 @@ impl Context {
         ));
     }
 
+    /// Handler for `console.log` invocations.
+    ///
+    /// If a test is currently running it takes the `args` array and stringifies
+    /// it and appends it to the current output of the test. Otherwise it passes
+    /// the arguments to the original `console.log` function, psased as
+    /// `original`.
     pub fn console_log(&self, original: &Function, args: &Array) {
         self.log(original, args, &self.current_log)
     }
 
+    /// Handler for `console.error` invocations.
+    ///
+    /// Works the same as `console_log` above.
     pub fn console_error(&self, original: &Function, args: &Array) {
         self.log(original, args, &self.current_error)
     }
@@ -217,6 +354,8 @@ impl Context {
 }
 
 impl Context {
+    /// Entry point for a test in wasm. The `#[wasm_bindgen_test]` macro
+    /// generates invocations of this method.
     pub fn execute(&self, name: &str, f: impl FnOnce()) {
         self.log_start(name);
         if let Some(filter) = &self.filter {

crates/test/src/rt/node.rs

@@ -6,12 +6,16 @@
 use wasm_bindgen::prelude::*;
 use js_sys::eval;
 
+/// Implementation of the `Formatter` trait for node.js
 pub struct Node {
+    /// Handle to node's imported `fs` module, imported dynamically because we
+    /// can't statically do it as it doesn't work in a browser.
     fs: NodeFs,
 }
 
 #[wasm_bindgen]
 extern {
+    // Type declarations for the `writeSync` function in node
     type NodeFs;
     #[wasm_bindgen(method, js_name = writeSync, structural)]
     fn write_sync(this: &NodeFs, fd: i32, data: &[u8]);
@@ -24,11 +28,15 @@ extern {
 }
 
 impl Node {
+    /// Attempts to create a new formatter for node.js, returning `None` if this
+    /// is executing in a browser and Node won't work.
     pub fn new() -> Option<Node> {
         if super::detect::is_browser() {
             return None
         }
 
+        // Use `eval` for now as a quick fix around static imports not working
+        // for dual browser/node support.
         let import = eval("require(\"fs\")").unwrap();
         Some(Node { fs: import.into() })
     }