Accept test and describe calls without a name, and settle skip and todo as Node does (#1371)

docs/content/limitations/builtin-apis/test-runner.mdx

@@ -4,3 +4,5 @@
 - **Suite and hook contexts:** a `describe` body may take its `SuiteContext`, which has `name`, `fullName`, `filePath`, `attempt`, `passed` and `diagnostic`. Hooks may take their context, as in Node: the suite's for `before` and `after`, the test's for `beforeEach`, `afterEach`, `t.before` and `t.after`. The `SuiteContext` `signal` and `log` members, and hook `done` callbacks, throw `SC2020`.
 - **Module-level hooks:** `before` hooks outside any suite run at their `before()` call, and the first top-level test waits for the async ones; `after` hooks outside any suite run after the last top-level test. Both get the root's context, named `<root>`. As in Node, a throwing root `before` fails each top-level test with its error, without running its body, and cancels the tests and suites inside each top-level suite. A throwing root `after` reports the entry file as a failed result, and the exit code is unchanged.
 - **Timeouts:** when a test's `{ timeout }` elapses, its running subtests are cancelled, as in Node. Subtests do not take their parent's `timeout` as their own, where Node's do, and the subtests of a timed-out test with `{ concurrency }` above 1 run to completion before the test reports. A body that keeps running after its timeout and calls `t.test` again runs that subtest, where Node reports it as not started.
+- **Thrown values:** a test that throws an object or array that is not an `Error`, such as `throw { code: 42 }`, reports `[object Object]` in the failing section, where Node prints the value as `util.inspect` renders it.
+- **Test names:** a test without a name, or with an empty one, takes its function's name or `<anonymous>`, as in Node, when the function's name is known at compile time: a function expression, a function declaration, or a `const` bound to an arrow or anonymous function. A name computed at run time, with a function the program received as a parameter, is used as given, so an empty one stays empty where Node uses the function's name. `test(fn)` and `test("", fn)` with such a function throw `SC2020`.

packages/compiler/ambient/scriptc-node-fallback.d.ts

@@ -3405,8 +3405,10 @@ declare module "node:test" {
     readonly tags: readonly string[];
     readonly assert: TestContextAssert;
     readonly mock: MockTracker;
-    test(name: string, fn?: TestFn): Promise<void>;
-    test(name: string, options: TestOptions, fn?: TestFn): Promise<void>;
+    test(name?: string, fn?: TestFn): Promise<void>;
+    test(name?: string, options?: TestOptions, fn?: TestFn): Promise<void>;
+    test(options?: TestOptions, fn?: TestFn): Promise<void>;
+    test(fn?: TestFn): Promise<void>;
     skip(message?: string): void;
     todo(message?: string): void;
     diagnostic(message: string): void;
@@ -3436,23 +3438,44 @@ declare module "node:test" {
     match(value: string, regExp: RegExp, message?: string): void;
     doesNotMatch(value: string, regExp: RegExp, message?: string): void;
   }
-  function test(name: string, fn?: TestFn): Promise<void>;
-  function test(name: string, options: TestOptions, fn?: TestFn): Promise<void>;
+  /* The name, options and function are each optional, as in Node: a test
+   * without a name takes the function's name, or "<anonymous>". */
+  function test(name?: string, fn?: TestFn): Promise<void>;
+  function test(name?: string, options?: TestOptions, fn?: TestFn): Promise<void>;
+  function test(options?: TestOptions, fn?: TestFn): Promise<void>;
+  function test(fn?: TestFn): Promise<void>;
   namespace test {
-    function skip(name: string, fn?: TestFn): Promise<void>;
-    function skip(name: string, options: TestOptions, fn?: TestFn): Promise<void>;
-    function todo(name: string, fn?: TestFn): Promise<void>;
-    function todo(name: string, options: TestOptions, fn?: TestFn): Promise<void>;
-    function only(name: string, fn?: TestFn): Promise<void>;
-    function only(name: string, options: TestOptions, fn?: TestFn): Promise<void>;
+    function skip(name?: string, fn?: TestFn): Promise<void>;
+    function skip(name?: string, options?: TestOptions, fn?: TestFn): Promise<void>;
+    function skip(options?: TestOptions, fn?: TestFn): Promise<void>;
+    function skip(fn?: TestFn): Promise<void>;
+    function todo(name?: string, fn?: TestFn): Promise<void>;
+    function todo(name?: string, options?: TestOptions, fn?: TestFn): Promise<void>;
+    function todo(options?: TestOptions, fn?: TestFn): Promise<void>;
+    function todo(fn?: TestFn): Promise<void>;
+    function only(name?: string, fn?: TestFn): Promise<void>;
+    function only(name?: string, options?: TestOptions, fn?: TestFn): Promise<void>;
+    function only(options?: TestOptions, fn?: TestFn): Promise<void>;
+    function only(fn?: TestFn): Promise<void>;
   }
   const it: typeof test;
-  function describe(name: string, fn?: SuiteFn): void;
-  function describe(name: string, options: TestOptions, fn?: SuiteFn): void;
+  function describe(name?: string, fn?: SuiteFn): void;
+  function describe(name?: string, options?: TestOptions, fn?: SuiteFn): void;
+  function describe(options?: TestOptions, fn?: SuiteFn): void;
+  function describe(fn?: SuiteFn): void;
   namespace describe {
-    function skip(name: string, fn?: SuiteFn): void;
-    function todo(name: string, fn?: SuiteFn): void;
-    function only(name: string, fn?: SuiteFn): void;
+    function skip(name?: string, fn?: SuiteFn): void;
+    function skip(name?: string, options?: TestOptions, fn?: SuiteFn): void;
+    function skip(options?: TestOptions, fn?: SuiteFn): void;
+    function skip(fn?: SuiteFn): void;
+    function todo(name?: string, fn?: SuiteFn): void;
+    function todo(name?: string, options?: TestOptions, fn?: SuiteFn): void;
+    function todo(options?: TestOptions, fn?: SuiteFn): void;
+    function todo(fn?: SuiteFn): void;
+    function only(name?: string, fn?: SuiteFn): void;
+    function only(name?: string, options?: TestOptions, fn?: SuiteFn): void;
+    function only(options?: TestOptions, fn?: SuiteFn): void;
+    function only(fn?: SuiteFn): void;
   }
   const suite: typeof describe;
   function before(fn: HookFn): void;

packages/compiler/src/frontend/lowering/lower-test.ts

@@ -90,12 +90,14 @@ function atStringOf(expr: ts.CallExpression): string {
   return `${sf.fileName}:${pos.line + 1}:${pos.character + 1}`;
 }
 
-/** One parsed { skip?, todo?, only? } options literal. `mode` is the
- * runtime literal (0 run / 1 skip / 2 todo), `msg` the directive message
- * expression ("" = none). */
+/** One parsed { skip?, todo?, only? } options literal. `skip` and `todo`
+ * are whether each is set (a string or true), and `skipMsg` and `todoMsg`
+ * the string messages (directiveOf settles them). */
 interface TestOptions {
-  mode: number;
-  msg: IrExpr | null;
+  skip: boolean;
+  skipMsg: IrExpr | null;
+  todo: boolean;
+  todoMsg: IrExpr | null;
   only: boolean;
   /** The { plan } count, -1 without one. */
   plan: number;
@@ -109,8 +111,10 @@ interface TestOptions {
 }
 
 const noOptions = (): TestOptions => ({
-  mode: 0,
-  msg: null,
+  skip: false,
+  skipMsg: null,
+  todo: false,
+  todoMsg: null,
   only: false,
   plan: -1,
   timeout: null,
@@ -118,6 +122,23 @@ const noOptions = (): TestOptions => ({
   pre: [],
 });
 
+/** The runtime mode (0 run / 1 skip / 2 todo) and directive message of a
+ * test or suite, as Node's Test constructor settles them: the skip and todo
+ * method twins override the options of the same name, skip wins over
+ * todo, and the message is the skip string, else the todo string. */
+function directiveOf(opts: TestOptions, methodMode: number, loc: SrcLoc): [IrExpr, IrExpr] {
+  if (methodMode === 1) {
+    opts.skip = true;
+    opts.skipMsg = null;
+  }
+  if (methodMode === 2) {
+    opts.todo = true;
+    opts.todoMsg = null;
+  }
+  const mode = opts.skip ? 1 : opts.todo ? 2 : 0;
+  return [numLit(mode, loc), opts.skipMsg ?? opts.todoMsg ?? strLit("", loc)];
+}
+
 /** Flags 8 and 16: a { timeout } and a { concurrency } are given. */
 const optionFlags = (opts: TestOptions): number =>
   (opts.only ? 4 : 0) | (opts.timeout ? 8 : 0) | (opts.concurrency ? 16 : 0);
@@ -168,15 +189,33 @@ function lowerTestOptions(
     // A shorthand entry's value is the binding of the same name.
     const init: ts.Expression = ts.isPropertyAssignment(prop) ? prop.initializer : key;
     if (name === "skip" || name === "todo") {
-      const mode = name === "skip" ? 1 : 2;
       const t = lowerer.typeOf(init);
       if (t.isStringLiteralType() || lowerer.mapTypeOf(t)?.kind === "string") {
-        out.mode = mode;
-        out.msg = optionValue(lowerer, out, lowerer.lowerExprExpecting(init, STRING), locOf(init));
-      } else if (init.kind === ts.SyntaxKind.TrueKeyword) {
-        out.mode = mode;
-      } else if (init.kind === ts.SyntaxKind.FalseKeyword) {
-        // explicit false: run normally
+        const msg = optionValue(
+          lowerer,
+          out,
+          lowerer.lowerExprExpecting(init, STRING),
+          locOf(init),
+        );
+        if (name === "skip") {
+          out.skip = true;
+          out.skipMsg = msg;
+        } else {
+          out.todo = true;
+          out.todoMsg = msg;
+        }
+      } else if (
+        init.kind === ts.SyntaxKind.TrueKeyword ||
+        init.kind === ts.SyntaxKind.FalseKeyword
+      ) {
+        const on = init.kind === ts.SyntaxKind.TrueKeyword;
+        if (name === "skip") {
+          out.skip = on;
+          out.skipMsg = null;
+        } else {
+          out.todo = on;
+          out.todoMsg = null;
+        }
       } else {
         lowerer.noLowering(
           `${what} with a non-literal ${name} value`,
@@ -284,6 +323,116 @@ function lowerBodyArg(
 /** The shared registration lowering behind test/it and their skip/todo/
  * only method twins — and, with `sub`, behind t.test. Argument shapes:
  * (name), (name, fn), (name, options, fn). */
+/** The `name` a function value has in JavaScript, when the compiler can
+ * tell: "" for an anonymous function expression, the declared or binding
+ * name otherwise; null for other expressions. */
+function staticFunctionName(lowerer: Lowerer, node: ts.Expression): string | null {
+  while (ts.isParenthesizedExpression(node)) node = node.expression;
+  if (ts.isArrowFunction(node)) return "";
+  if (ts.isFunctionExpression(node)) return node.name?.text ?? "";
+  if (!ts.isIdentifier(node)) return null;
+  const symbol = lowerer.resolveValueSymbol(node);
+  if (!symbol) return null;
+  for (const decl of lowerer.checker.declarationsOf(symbol)) {
+    if (ts.isFunctionDeclaration(decl)) return decl.name?.text ?? "default";
+    if (ts.isVariableDeclaration(decl) && ts.isIdentifier(decl.name) && decl.initializer) {
+      let init = decl.initializer;
+      while (ts.isParenthesizedExpression(init)) init = init.expression;
+      if (ts.isFunctionExpression(init) && init.name) return init.name.text;
+      if (ts.isArrowFunction(init) || ts.isFunctionExpression(init)) return decl.name.text;
+    }
+  }
+  return null;
+}
+
+/** Node's (name?, options?, fn?) argument forms (`Test#createSubtest`): a
+ * leading function is the body, a leading object the options, otherwise
+ * the first argument is the name and a following object the options. */
+function splitTestArgs(
+  lowerer: Lowerer,
+  expr: ts.CallExpression,
+  what: string,
+): { nameNode?: ts.Expression; optionsNode?: ts.Expression; fnNode?: ts.Expression } {
+  const args = expr.arguments;
+  const isFn = (a: ts.Expression): boolean =>
+    lowerer.checker.getCallSignatures(lowerer.typeOf(a)).length > 0;
+  const isOptions = (a: ts.Expression): boolean => ts.isObjectLiteralExpression(a);
+  const unsupported = (): never =>
+    lowerer.noLowering(
+      `${what} with these arguments`,
+      expr,
+      "the supported forms are (name?, options?, fn?) with a string name, an options object literal, and a function",
+    );
+  if (args.length > 3 || args.some(ts.isSpreadElement)) unsupported();
+  const out: { nameNode?: ts.Expression; optionsNode?: ts.Expression; fnNode?: ts.Expression } = {};
+  let i = 0;
+  const next = args[i];
+  if (next !== undefined && !isFn(next) && !isOptions(next)) {
+    out.nameNode = next;
+    i++;
+  }
+  const options = args[i];
+  if (options !== undefined && isOptions(options)) {
+    out.optionsNode = options;
+    i++;
+  }
+  const fn = args[i];
+  if (fn !== undefined && isFn(fn)) {
+    out.fnNode = fn;
+    i++;
+  }
+  if (i < args.length) unsupported();
+  return out;
+}
+
+/** The test's name, as Node's Test constructor completes it: a missing or
+ * empty name takes the function's name, or "<anonymous>". */
+function lowerTestName(
+  lowerer: Lowerer,
+  expr: ts.CallExpression,
+  what: string,
+  nameNode: ts.Expression | undefined,
+  fnNode: ts.Expression | undefined,
+  loc: SrcLoc,
+): IrExpr {
+  const fallback = (): string => {
+    const fnName = fnNode ? staticFunctionName(lowerer, fnNode) : "";
+    if (fnName === null) {
+      lowerer.noLowering(
+        `${what} without a name, with a function whose name is not known at compile time`,
+        fnNode ?? expr,
+        "give the test a name, or pass a function expression or a declared function",
+      );
+    }
+    return fnName || "<anonymous>";
+  };
+  if (!nameNode) return strLit(fallback(), loc);
+  const name = lowerer.lowerExprExpecting(nameNode, STRING);
+  if (name.kind === "strLit") return name.value === "" ? strLit(fallback(), loc) : name;
+  // A name computed at run time: when the function's own name is not known
+  // at compile time (a function passed in as a parameter, say), the name
+  // is used as given, and an empty one stays empty (the limitations page
+  // notes it). Refusing here would refuse every helper that registers
+  // tests for its callers.
+  if (fnNode && staticFunctionName(lowerer, fnNode) === null) return name;
+  const slot = lowerer.declareHiddenLocal("%testName", STRING);
+  const value = varRef(slot.id, STRING, loc);
+  return {
+    kind: "seqExpr",
+    stmts: [{ kind: "varDecl", localId: slot.id, init: name, loc }],
+    result: {
+      kind: "ternary",
+      cond: { kind: "strEq", negated: false, left: value, right: strLit("", loc), type: BOOL, loc },
+      then: strLit(fallback(), loc),
+      else_: value,
+      type: STRING,
+      loc,
+    },
+    type: STRING,
+    loc,
+  };
+}
+
 function lowerRegistration(
   lowerer: Lowerer,
   expr: ts.CallExpression,
@@ -293,21 +442,11 @@ function lowerRegistration(
   sub: IrExpr | null, // the lowered TestContext receiver (t.test)
 ): IrExpr {
   if (sub === null) requireStatementPosition(lowerer, expr, what);
-  const args = expr.arguments;
-  if (args.length < 1 || args.length > 3) {
-    lowerer.noLowering(
-      `${what} with ${args.length} arguments`,
-      expr,
-      "the supported forms are (name), (name, fn), and (name, options, fn)",
-    );
-  }
-  let name = lowerer.lowerExprExpecting(args[0]!, STRING);
+  const { nameNode, optionsNode, fnNode } = splitTestArgs(lowerer, expr, what);
+  let name = lowerTestName(lowerer, expr, what, nameNode, fnNode, loc);
   let opts = noOptions();
-  let fnNode: ts.Expression | undefined;
-  if (args.length === 2) fnNode = args[1];
-  if (args.length === 3) {
-    opts = lowerTestOptions(lowerer, args[1]!, what, false);
-    fnNode = args[2];
+  if (optionsNode) {
+    opts = lowerTestOptions(lowerer, optionsNode, what, false);
     // The name runs before the option values.
     if (opts.pre.length > 0 && name.kind !== "strLit") {
       const slot = lowerer.declareHiddenLocal("%testName", STRING);
@@ -315,11 +454,9 @@ function lowerRegistration(
       name = { kind: "varRef", localId: slot.id, type: STRING, loc };
     }
   }
-  if (methodMode === 1 || methodMode === 2) opts.mode = methodMode;
   if (methodMode === 3) opts.only = true;
   const at = strLit(atStringOf(expr), loc);
-  const mode = numLit(opts.mode, loc);
-  const msg = opts.msg ?? strLit("", loc);
+  const [mode, msg] = directiveOf(opts, methodMode, loc);
   const flagBits = optionFlags(opts);
   const planTimeout = [
     numLit(opts.plan, loc),
@@ -387,27 +524,21 @@ function lowerSuite(
   methodMode: number,
 ): IrExpr {
   requireStatementPosition(lowerer, expr, what);
-  const args = expr.arguments;
-  if (args.length < 2 || args.length > 3) {
-    lowerer.noLowering(
-      `${what} with ${args.length} arguments`,
-      expr,
-      "the supported forms are (name, fn) and (name, options, fn)",
-    );
-  }
-  let name = lowerer.lowerExprExpecting(args[0]!, STRING);
+  const split = splitTestArgs(lowerer, expr, what);
+  let name = lowerTestName(lowerer, expr, what, split.nameNode, split.fnNode, loc);
   let opts = noOptions();
-  let fnNode = args[1]!;
-  if (args.length === 3) {
-    opts = lowerTestOptions(lowerer, args[1]!, what, true);
-    fnNode = args[2]!;
+  if (split.optionsNode) {
+    opts = lowerTestOptions(lowerer, split.optionsNode, what, true);
     if (opts.pre.length > 0 && name.kind !== "strLit") {
       const slot = lowerer.declareHiddenLocal("%testName", STRING);
       opts.pre.unshift({ kind: "varDecl", localId: slot.id, init: name, loc });
       name = { kind: "varRef", localId: slot.id, type: STRING, loc };
     }
   }
-  if (methodMode === 1 || methodMode === 2) opts.mode = methodMode;
+  const fnNode = split.fnNode;
+  if (!fnNode) {
+    lowerer.noLowering(`${what} without a body`, expr, "give the suite a body function");
+  }
   const cb = lowerer.lowerExpr(fnNode);
   // The body may take the SuiteContext (flag 2, as test bodies take theirs).
   if (
@@ -429,8 +560,7 @@ function lowerSuite(
     );
   }
   const at = strLit(atStringOf(expr), loc);
-  const mode = numLit(opts.mode, loc);
-  const msg = opts.msg ?? strLit("", loc);
+  const [mode, msg] = directiveOf(opts, methodMode, loc);
   // describe.only and { only: true } mark the suite (flag 4, as for tests).
   const flags = numLit(
     optionFlags(opts) | (methodMode === 3 ? 4 : 0) | (cb.type.params.length === 1 ? 2 : 0),

tests/fixtures/node-test/cases/test-name-forms.ts

@@ -0,0 +1,55 @@
+// The (name?, options?, fn?) forms of test, it, describe and t.test, and
+// Node's default names: a missing or empty name takes the function's name,
+// or "<anonymous>". A helper may register tests with computed names and
+// functions it receives as parameters. With both skip and todo, skip wins, in either order and
+// against the todo method twins.
+import { describe, it, test } from "node:test";
+
+function declaredTest(): void {}
+const boundArrow = (): void => {};
+const emptyName = process.argv.length > 99 ? "never" : "";
+
+// A helper that registers tests for its callers: the name is computed and
+// the function arrives as a parameter.
+function registerCase(label: string, body: () => void): void {
+  test(`case ${label}`, body);
+}
+registerCase("one", () => {});
+registerCase("two", declaredTest);
+
+test(function namedExpression() {});
+test(() => {});
+test(declaredTest);
+test(boundArrow);
+test("", boundArrow);
+test(emptyName, declaredTest);
+test({ skip: "options first" }, () => {});
+test({ todo: true });
+test();
+test("name and options", { skip: true });
+
+describe(function namedSuite() {
+  it(() => {});
+  it({ todo: "later" }, function insideTodo() {});
+});
+describe({ skip: "whole suite" }, () => {
+  it("not run", () => {});
+});
+
+test("subtests", async (t) => {
+  await t.test(function namedSub() {});
+  await t.test(() => {});
+  await t.test({ skip: true }, () => {});
+});
+
+test("skip then todo", { skip: "skip message", todo: "todo message" }, () => {});
+test("todo then skip", { todo: "todo message", skip: true }, () => {});
+test.todo("todo twin with skip", { skip: true }, () => {});
+it.skip("skip twin with todo", { todo: true }, () => {});
+// The twins override the option of the same name, and the message is the
+// skip string, else the todo string.
+test.skip("skip twin with a skip message", { skip: "option message" }, () => {});
+it.todo("todo twin with a todo message", { todo: "option message" }, () => {});
+it.skip("skip twin with a todo message", { todo: "todo message" }, () => {});
+test.todo("todo twin with a skip message", { skip: "skip message" }, () => {});
+test("skip false with a todo message", { skip: false, todo: "todo message" }, () => {});