Update CLI for unit_test_runner.ts (#4352)

* drop server guard before unit test result check

To prevent cascading test failures when js_unit_test http server
guard is dropped before asserting that tests were successful.
This is really a band-aid and doesn't solve underlying issue with
http server.

* Update CLI for unit_test_runner.ts

* Change cli/js/tests/unit_test_runner.ts command line interface to work in 3
  modes:
  - "one-off" - run tests that match permissions of currently running
    process
  - "master" - run tests for all possible permission combinations, by
   spawning subprocesses running in "worker" mode and communicating via
   TCP socket; requires elevated permissions
  - "worker" - run tests for set of permissions provided by CLI arg;
  requires elevated permissions to setup TCP connection to "master";
  after initial setup process drops permissions to given set

* Support filtering of tests by string passed after "--" CLI arg

* Update cli/js/tests/README.md

cli/js/tests/README.md

@@ -37,11 +37,58 @@ ways:
 - sanitization of async ops - ensuring that tests don't leak async ops by
   ensuring that all started async ops are done before test finishes
 
-`unit_test_runner.ts` is main script used to run unit tests.
+## Running tests
+
+`unit_test_runner.ts` is the main script used to run unit tests.
 
 Runner discoveres required permissions combinations by loading
 `cli/js/tests/unit_tests.ts` and going through all registered instances of
-`unitTest`. For each discovered permission combination a new Deno process is
-created with respective `--allow-*` flags which loads
-`cli/js/tests/unit_tests.ts` and executes all `unitTest` that match runtime
-permissions.
+`unitTest`.
+
+There are three ways to run `unit_test_runner.ts`:
+
+- run tests matching current process permissions
+
+```
+// run tests that don't require any permissions
+target/debug/deno unit_test_runner.ts
+
+// run tests with "net" permission
+target/debug/deno --allow-net unit_test_runner.ts
+
+target/debug/deno --allow-net --allow-read unit_test_runner.ts
+```
+
+- run all tests - "master" mode, that spawns worker processes for each
+  discovered permission combination:
+
+```
+target/debug/deno -A unit_test_runner.ts --master
+```
+
+By default all output of worker processes is discarded; for debug purposes
+`--verbose` flag can be provided to preserve output from worker
+
+```
+target/debug/deno -A unit_test_runner.ts --master --verbose
+```
+
+- "worker" mode; communicates with parent using TCP socket on provided address;
+  after initial setup drops permissions to specified set. It shouldn't be used
+  directly, only be "master" process.
+
+```
+target/debug/deno -A unit_test_runner.ts --worker --addr=127.0.0.1:4500 --perms=net,write,run
+```
+
+### Filtering
+
+Runner supports basic test filtering by name:
+
+```
+target/debug/deno unit_test_runner.ts -- netAccept
+
+target/debug/deno -A unit_test_runner.ts --master -- netAccept
+```
+
+Filter string must be specified after "--" argument

cli/js/tests/test_util.ts

@@ -13,6 +13,7 @@ export {
   fail
 } from "../../../std/testing/asserts.ts";
 export { readLines } from "../../../std/io/bufio.ts";
+export { parse as parseArgs } from "../../../std/flags/mod.ts";
 
 export interface Permissions {
   read: boolean;

cli/js/tests/unit_test_runner.ts

@@ -2,13 +2,13 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 import "./unit_tests.ts";
 import {
-  assert,
   readLines,
   permissionCombinations,
   Permissions,
   registerUnitTests,
   SocketReporter,
-  fmtPerms
+  fmtPerms,
+  parseArgs
 } from "./test_util.ts";
 
 interface PermissionSetTestResult {
@@ -44,17 +44,15 @@ async function dropWorkerPermissions(
   }
 }
 
-async function workerRunnerMain(args: string[]): Promise<void> {
-  const addrArg = args.find(e => e.includes("--addr"));
-  assert(typeof addrArg === "string", "Missing --addr argument");
-  const addrStr = addrArg.split("=")[1];
+async function workerRunnerMain(
+  addrStr: string,
+  permsStr: string,
+  filter?: string
+): Promise<void> {
   const [hostname, port] = addrStr.split(":");
   const addr = { hostname, port: Number(port) };
 
   let perms: Deno.PermissionName[] = [];
-  const permsArg = args.find(e => e.includes("--perms"));
-  assert(typeof permsArg === "string", "Missing --perms argument");
-  const permsStr = permsArg.split("=")[1];
   if (permsStr.length > 0) {
     perms = permsStr.split(",") as Deno.PermissionName[];
   }
@@ -69,13 +67,19 @@ async function workerRunnerMain(args: string[]): Promise<void> {
   await Deno.runTests({
     failFast: false,
     exitOnFail: false,
-    reporter: socketReporter
+    reporter: socketReporter,
+    only: filter
   });
   // Notify parent process we're done
   socketReporter.close();
 }
 
-function spawnWorkerRunner(addr: string, perms: Permissions): Deno.Process {
+function spawnWorkerRunner(
+  verbose: boolean,
+  addr: string,
+  perms: Permissions,
+  filter?: string
+): Deno.Process {
   // run subsequent tests using same deno executable
   const permStr = Object.keys(perms)
     .filter((permName): boolean => {
@@ -88,25 +92,33 @@ function spawnWorkerRunner(addr: string, perms: Permissions): Deno.Process {
     "run",
     "-A",
     "cli/js/tests/unit_test_runner.ts",
-    "--",
     "--worker",
     `--addr=${addr}`,
     `--perms=${permStr}`
   ];
 
+  if (filter) {
+    args.push("--");
+    args.push(filter);
+  }
+
+  const ioMode = verbose ? "inherit" : "null";
+
   const p = Deno.run({
     args,
-    stdin: "null",
-    stdout: "piped",
-    stderr: "null"
+    stdin: ioMode,
+    stdout: ioMode,
+    stderr: ioMode
   });
 
   return p;
 }
 
 async function runTestsForPermissionSet(
+  verbose: boolean,
   reporter: Deno.ConsoleTestReporter,
-  perms: Permissions
+  perms: Permissions,
+  filter?: string
 ): Promise<PermissionSetTestResult> {
   const permsFmt = fmtPerms(perms);
   console.log(`Running tests for: ${permsFmt}`);
@@ -114,7 +126,7 @@ async function runTestsForPermissionSet(
   const addrStr = `${addr.hostname}:${addr.port}`;
   const workerListener = Deno.listen(addr);
 
-  const workerProcess = spawnWorkerRunner(addrStr, perms);
+  const workerProcess = spawnWorkerRunner(verbose, addrStr, perms, filter);
 
   // Wait for worker subprocess to go online
   const conn = await workerListener.accept();
@@ -182,7 +194,10 @@ async function runTestsForPermissionSet(
   };
 }
 
-async function masterRunnerMain(): Promise<void> {
+async function masterRunnerMain(
+  verbose: boolean,
+  filter?: string
+): Promise<void> {
   console.log(
     "Discovered permission combinations for tests:",
     permissionCombinations.size
@@ -196,7 +211,12 @@ async function masterRunnerMain(): Promise<void> {
   const consoleReporter = new Deno.ConsoleTestReporter();
 
   for (const perms of permissionCombinations.values()) {
-    const result = await runTestsForPermissionSet(consoleReporter, perms);
+    const result = await runTestsForPermissionSet(
+      verbose,
+      consoleReporter,
+      perms,
+      filter
+    );
     testResults.add(result);
   }
 
@@ -224,16 +244,87 @@ async function masterRunnerMain(): Promise<void> {
   console.log("Unit tests passed");
 }
 
+const HELP = `Unit test runner
+
+Run tests matching current process permissions:
+
+  deno --allow-write unit_test_runner.ts
+
+  deno --allow-net --allow-hrtime unit_test_runner.ts
+
+  deno --allow-write unit_test_runner.ts -- testWriteFile
+
+Run "master" process that creates "worker" processes
+for each discovered permission combination:
+
+  deno -A unit_test_runner.ts --master
+
+Run worker process for given permissions:
+
+  deno -A unit_test_runner.ts --worker --perms=net,read,write --addr=127.0.0.1:4500
+
+
+OPTIONS:
+  --master 
+    Run in master mode, spawning worker processes for
+    each discovered permission combination
+    
+  --worker 
+    Run in worker mode, requires "perms" and "addr" flags,
+    should be run with "-A" flag; after setup worker will
+    drop permissions to required set specified in "perms"
+
+  --perms=<perm_name>...
+    Set of permissions this process should run tests with,
+
+  --addr=<addr>
+    Address of TCP socket for reporting
+
+ARGS:
+  -- <filter>... 
+    Run only tests with names matching filter, must
+    be used after "--"
+`;
+
+function assertOrHelp(expr: unknown): asserts expr {
+  if (!expr) {
+    console.log(HELP);
+    Deno.exit(1);
+  }
+}
+
 async function main(): Promise<void> {
-  const args = Deno.args;
+  const args = parseArgs(Deno.args, {
+    boolean: ["master", "worker", "verbose"],
+    "--": true
+  });
 
-  const isWorker = args.includes("--worker");
-
-  if (isWorker) {
-    return await workerRunnerMain(args);
+  if (args.help) {
+    console.log(HELP);
+    return;
   }
 
-  return await masterRunnerMain();
+  const filter = args["--"][0];
+
+  // Master mode
+  if (args.master) {
+    return await masterRunnerMain(args.verbose, filter);
+  }
+
+  // Worker mode
+  if (args.worker) {
+    assertOrHelp(typeof args.addr === "string");
+    assertOrHelp(typeof args.perms === "string");
+    return await workerRunnerMain(args.addr, args.perms, filter);
+  }
+
+  // Running tests matching current process permissions
+  await registerUnitTests();
+  await Deno.runTests({
+    failFast: false,
+    exitOnFail: true,
+    only: filter
+  });
 }
 
 main();

cli/tests/integration_tests.rs

@@ -274,12 +274,13 @@ fn js_unit_tests() {
     .arg("--reload")
     .arg("-A")
     .arg("cli/js/tests/unit_test_runner.ts")
+    .arg("--master")
     .spawn()
     .expect("failed to spawn script");
   let status = deno.wait().expect("failed to wait for the child process");
+  drop(g);
   assert_eq!(Some(0), status.code());
   assert!(status.success());
-  drop(g);
 }
 
 #[test]