fix(cli): reject a wrong --wait and --json with --markdown, and send the usage to stderr

cli/CHANGELOG.md

@@ -11,6 +11,10 @@ Format based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). The CL
 ### Fixed
 - The markdown report escapes the message of an error in the New errors and Fixed errors lists of `--baseline`, as it does in its Errors and warnings list and in its tables. They printed the message as it came, so GitHub dropped the `<head>` of `1 blocking script(s) in <head>` there, and a line break in a message could start a heading or another list item.
 - The human and markdown reports say `not supported` and why for a snippet whose API the browser lacks, a result with the `unsupported` status. They printed it as a metric without a value: `undefined` as its value, and `· undefined` as its status in the markdown table.
+- `--wait` with a value that is not a whole number of milliseconds, such as `--wait abc`, exits with `2` before the browser opens. It ran the snippets without any wait and said nothing, so a typo measured the page too early.
+- `--json` with `--markdown` exits with `2`: it printed the markdown report, which a script that asked for JSON could not parse.
+- Without a URL or `--matrix`, the usage goes to stderr, as every other usage error, and stdout stays empty for a pipe or a redirect. `--help` still prints it to stdout.
+- An unknown `--workflow` lists the workflows there are.
 
 ---
 

cli/src/bin.js

@@ -212,15 +212,19 @@ async function main() {
   const url = positionals[0];
   const matrixPath = values.matrix;
   if (url && matrixPath) fail("Use a URL or --matrix, not both");
-  if (!url && !matrixPath) {
-    process.stdout.write(USAGE);
-    process.exit(2);
-  }
+  // To stderr, as every usage error: a pipe or a redirect of stdout gets no usage text as a report
+  if (!url && !matrixPath) fail(USAGE.trimEnd());
   // Without the URL itself, which can carry a token in its query, as a header can
   if (url && !isHttpUrl(url)) fail("The URL must be an http or https URL, such as https://example.com or http://localhost:3000");
   if (matrixPath && values.viewport) fail("--viewport does not apply to --matrix: set the viewport of each device in the file");
 
-  const waitMs = values.wait ? Number(values.wait) : 3000;
+  if (values.json && values.markdown) fail("Use --json or --markdown, not both");
+
+  // Number("") is 0 and Number("abc") is NaN, which the wait takes as no wait: only digits are a wait
+  if (values.wait != null && !/^\d+$/.test(values.wait)) {
+    fail(`Invalid --wait "${values.wait}". Use a number of milliseconds, such as 3000, or 0 for no wait`);
+  }
+  const waitMs = values.wait != null ? Number(values.wait) : 3000;
   const viewportName = values.viewport ?? "mobile";
   const viewport = VIEWPORT_PRESETS[viewportName];
   if (!viewport) {
@@ -315,7 +319,7 @@ async function main() {
   // What each page load runs, a snippet or a workflow, under the conditions of the load
   const items = values.snippet ? buildSnippetItem(values) : null;
   const workflow = values.snippet ? null : WORKFLOWS[values.workflow ?? "core-web-vitals"];
-  if (!values.snippet && !workflow) fail(`Unknown workflow: ${values.workflow}`);
+  if (!values.snippet && !workflow) fail(`Unknown workflow: "${values.workflow}". Choose from: ${Object.keys(WORKFLOWS).join(", ")}`);
   // A budget of a metric that no snippet of the run measures could never pass: a usage error
   const [unmeasured] = unmeasuredMetrics(budgets, items ? [items[0].path] : workflow.steps.map((step) => step.path));
   if (unmeasured) fail(budgetUsage(unmeasured, items ? `--snippet ${values.snippet}` : `the ${workflow.id} workflow`));

cli/tests/e2e/options.test.js

@@ -34,3 +34,35 @@ describe("--version", () => {
     expect(stdout).toMatch(/^\s+-v, --version\s+Show the version$/m);
   }, 30000);
 });
+
+// Each of these stops before the browser opens, so the URL is never loaded
+describe("a wrong use of the options", () => {
+  const URL = "http://localhost:9";
+
+  it.each(["abc", "-1", "1s", ""])('--wait "%s" is a usage error, not a run without a wait', async (wait) => {
+    const { code, stdout, stderr } = await cli(URL, `--wait=${wait}`);
+    expect(code).toBe(2);
+    expect(stdout).toBe("");
+    expect(stderr).toBe(`Invalid --wait "${wait}". Use a number of milliseconds, such as 3000, or 0 for no wait\n`);
+  }, 30000);
+
+  it("--json and --markdown together are a usage error, not a markdown report", async () => {
+    const { code, stdout, stderr } = await cli(URL, "--json", "--markdown");
+    expect(code).toBe(2);
+    expect(stdout).toBe("");
+    expect(stderr).toBe("Use --json or --markdown, not both\n");
+  }, 30000);
+
+  it("without a URL, the usage goes to stderr, so a pipe or a redirect of stdout stays empty", async () => {
+    const { code, stdout, stderr } = await cli();
+    expect(code).toBe(2);
+    expect(stdout).toBe("");
+    expect(stderr).toMatch(/^webperf-snippets <url> \[options\]\n/);
+  }, 30000);
+
+  it("an unknown workflow lists the ones there are", async () => {
+    const { code, stderr } = await cli(URL, "--workflow", "everything");
+    expect(code).toBe(2);
+    expect(stderr).toMatch(/^Unknown workflow: "everything"\. Choose from: .*\bcore-web-vitals\b.*\baudit\b/);
+  }, 30000);
+});