fix(testing): top-level hook outside describe no longer inflates step count

testing/_test_suite.ts

@@ -69,12 +69,27 @@ export class TestSuiteInternal<T> implements TestSuite<T> {
   protected describe: DescribeDefinition<T>;
   protected steps: (TestSuiteInternal<T> | ItDefinition<T>)[];
   protected hasOnlyStep: boolean;
+  /**
+   * Whether this is the synthetic "global" suite created when a top-level
+   * `beforeAll`/`afterAll`/`beforeEach`/`afterEach` is called outside any
+   * `describe`. Synthetic suites are only registered with `Deno.test` if a
+   * top-level `it()` is also added; otherwise their hooks are inherited by
+   * child describes promoted to top-level `Deno.test`s.
+   */
+  protected isSynthetic: boolean;
+  /**
+   * For a child of a synthetic global suite, this points back to the synthetic
+   * suite so its hooks can be invoked around the child's tests at run time.
+   */
+  protected syntheticParent: TestSuiteInternal<T> | null;
   #registeredOptions: Deno.TestDefinition | undefined;
 
-  constructor(describe: DescribeDefinition<T>) {
+  constructor(describe: DescribeDefinition<T>, isSynthetic = false) {
     this.describe = describe;
     this.steps = [];
     this.hasOnlyStep = false;
+    this.isSynthetic = isSynthetic;
+    this.syntheticParent = null;
 
     const { suite } = describe;
     if (suite && !TestSuiteInternal.suites.has(suite.symbol)) {
@@ -138,41 +153,82 @@ export class TestSuiteInternal<T> implements TestSuite<T> {
       }
     }
 
-    if (testSuite) {
+    if (testSuite && testSuite.isSynthetic) {
+      // Promote: a child describe of the synthetic global is registered as its
+      // own top-level Deno.test rather than as a step of the global suite. The
+      // child inherits the global's hooks at run time via syntheticParent.
+      this.syntheticParent = testSuite;
+      this.registerAsDenoTest();
+    } else if (testSuite) {
       TestSuiteInternal.addStep(testSuite, this);
-    } else {
-      const {
-        name,
-        ignore,
-        permissions,
-        sanitizeExit = globalSanitizersState.sanitizeExit,
-        sanitizeOps = globalSanitizersState.sanitizeOps,
-        sanitizeResources = globalSanitizersState.sanitizeResources,
-      } = describe;
-      let { only } = describe;
-      if (!ignore && this.hasOnlyStep) {
-        only = true;
-      }
-      const options: Deno.TestDefinition = {
-        name,
-        fn: async (t) => {
-          TestSuiteInternal.runningCount++;
-          try {
-            const context = {} as T;
-            const { beforeAll } = this.describe;
+    } else if (!this.isSynthetic) {
+      this.registerAsDenoTest();
+    }
+    // Synthetic suites without a parent are not registered eagerly. They are
+    // registered lazily by `addStep` when a top-level `it()` is added.
+  }
+
+  /** Builds the Deno.test options for this suite and registers them. */
+  protected registerAsDenoTest() {
+    if (this.#registeredOptions) return;
+    const {
+      name,
+      ignore,
+      permissions,
+      sanitizeExit = globalSanitizersState.sanitizeExit,
+      sanitizeOps = globalSanitizersState.sanitizeOps,
+      sanitizeResources = globalSanitizersState.sanitizeResources,
+    } = this.describe;
+    let { only } = this.describe;
+    if (!ignore && this.hasOnlyStep) {
+      only = true;
+    }
+    const options: Deno.TestDefinition = {
+      name,
+      fn: async (t) => {
+        TestSuiteInternal.runningCount++;
+        try {
+          const context = {} as T;
+          const parent = this.syntheticParent;
+          if (parent) {
+            const { beforeAll } = parent.describe;
             if (typeof beforeAll === "function") {
               await beforeAll.call(context);
             } else if (beforeAll) {
               for (const hook of beforeAll) {
                 await hook.call(context);
               }
             }
-            try {
-              TestSuiteInternal.active.push(this.symbol);
-              await TestSuiteInternal.run(this, context, t);
-            } finally {
+          }
+          const { beforeAll } = this.describe;
+          if (typeof beforeAll === "function") {
+            await beforeAll.call(context);
+          } else if (beforeAll) {
+            for (const hook of beforeAll) {
+              await hook.call(context);
+            }
+          }
+          try {
+            if (parent) {
+              TestSuiteInternal.active.push(parent.symbol);
+            }
+            TestSuiteInternal.active.push(this.symbol);
+            await TestSuiteInternal.run(this, context, t);
+          } finally {
+            TestSuiteInternal.active.pop();
+            if (parent) {
               TestSuiteInternal.active.pop();
-              const { afterAll } = this.describe;
+            }
+            const { afterAll } = this.describe;
+            if (typeof afterAll === "function") {
+              await afterAll.call(context);
+            } else if (afterAll) {
+              for (const hook of afterAll) {
+                await hook.call(context);
+              }
+            }
+            if (parent) {
+              const { afterAll } = parent.describe;
               if (typeof afterAll === "function") {
                 await afterAll.call(context);
               } else if (afterAll) {
@@ -181,31 +237,31 @@ export class TestSuiteInternal<T> implements TestSuite<T> {
                 }
               }
             }
-          } finally {
-            TestSuiteInternal.runningCount--;
           }
-        },
-      };
-      if (ignore !== undefined) {
-        options.ignore = ignore;
-      }
-      if (only !== undefined) {
-        options.only = only;
-      }
-      if (permissions !== undefined) {
-        options.permissions = permissions;
-      }
-      if (sanitizeExit !== undefined) {
-        options.sanitizeExit = sanitizeExit;
-      }
-      if (sanitizeOps !== undefined) {
-        options.sanitizeOps = sanitizeOps;
-      }
-      if (sanitizeResources !== undefined) {
-        options.sanitizeResources = sanitizeResources;
-      }
-      this.#registeredOptions = TestSuiteInternal.registerTest(options);
+        } finally {
+          TestSuiteInternal.runningCount--;
+        }
+      },
+    };
+    if (ignore !== undefined) {
+      options.ignore = ignore;
+    }
+    if (only !== undefined) {
+      options.only = only;
     }
+    if (permissions !== undefined) {
+      options.permissions = permissions;
+    }
+    if (sanitizeExit !== undefined) {
+      options.sanitizeExit = sanitizeExit;
+    }
+    if (sanitizeOps !== undefined) {
+      options.sanitizeOps = sanitizeOps;
+    }
+    if (sanitizeResources !== undefined) {
+      options.sanitizeResources = sanitizeResources;
+    }
+    this.#registeredOptions = TestSuiteInternal.registerTest(options);
   }
 
   /** Stores how many test suites are executing. */
@@ -289,6 +345,17 @@ export class TestSuiteInternal<T> implements TestSuite<T> {
     suite: TestSuiteInternal<T>,
     step: TestSuiteInternal<T> | ItDefinition<T>,
   ) {
+    // When adding a top-level `it()` to the synthetic global suite, the global
+    // needs to become a real `Deno.test` so the test has a place to run.
+    // Child `describe`s are promoted at construction time and never reach
+    // `addStep` with the synthetic suite as their parent.
+    if (
+      suite.isSynthetic && !suite.#registeredOptions &&
+      !(step instanceof TestSuiteInternal)
+    ) {
+      suite.registerAsDenoTest();
+    }
+
     if (!suite.hasOnlyStep) {
       if (step instanceof TestSuiteInternal) {
         if (step.hasOnlyStep || step.describe.only) {

testing/bdd.ts

@@ -834,7 +834,7 @@ function addHook<T>(
     TestSuiteInternal.current = new TestSuiteInternal({
       name: "global",
       [name]: fn,
-    });
+    }, true);
   } else {
     TestSuiteInternal.setHook(TestSuiteInternal.current!, name, fn);
   }

testing/bdd_test.ts

@@ -123,6 +123,162 @@ Deno.test("beforeAll(), afterAll(), beforeEach() and afterEach()", async () => {
   assertSpyCalls(afterEachFn, 2);
 });
 
+Deno.test(
+  "top-level beforeAll() with only nested describe() does not add an extra step",
+  async () => {
+    using test = stub(Deno, "test");
+    const fns = [spy(), spy()] as const;
+    const { beforeAllFn, afterAllFn, beforeEachFn, afterEachFn } = hookFns();
+
+    const context = new TestContext("the describe");
+    try {
+      beforeAll(beforeAllFn);
+      afterAll(afterAllFn);
+      beforeEach(beforeEachFn);
+      afterEach(afterEachFn);
+
+      describe("the describe", () => {
+        it({ name: "test 1", fn: fns[0] });
+        it({ name: "test 2", fn: fns[1] });
+      });
+
+      // Only one Deno.test should be registered, named after the user's
+      // describe — not a synthetic "global" wrapper. Without the fix this
+      // is called twice (once for "global", once as a step), inflating the
+      // step count reported by `deno test`.
+      assertSpyCalls(test, 1);
+      const options = test.calls[0]?.args[0] as Deno.TestDefinition;
+      assertEquals(Object.keys(options).sort(), ["fn", "name"]);
+      assertEquals(options.name, "the describe");
+
+      const result = options.fn(context);
+      assertStrictEquals(Promise.resolve(result), result);
+      assertEquals(await result, undefined);
+      // Only the two user tests should appear as steps; no extra wrapping step.
+      assertSpyCalls(context.spies.step, 2);
+    } finally {
+      TestSuiteInternal.reset();
+    }
+
+    assertSpyCalls(fns[0], 1);
+    assertSpyCalls(fns[1], 1);
+
+    // Top-level hooks still run around the nested tests.
+    assertSpyCalls(beforeAllFn, 1);
+    assertSpyCalls(afterAllFn, 1);
+    assertSpyCalls(beforeEachFn, 2);
+    assertSpyCalls(afterEachFn, 2);
+  },
+);
+
+Deno.test(
+  "top-level beforeAll() with only nested describe() still wires up hooks when describe has its own hooks",
+  async () => {
+    using test = stub(Deno, "test");
+    const fns = [spy(), spy()] as const;
+    const globalBeforeAll = spy();
+    const globalAfterAll = spy();
+    const globalBeforeEach = spy();
+    const globalAfterEach = spy();
+    const localBeforeAll = spy();
+    const localAfterAll = spy();
+    const localBeforeEach = spy();
+    const localAfterEach = spy();
+
+    const order: string[] = [];
+    const trackOrder = (label: string) => () => {
+      order.push(label);
+    };
+
+    const context = new TestContext("d");
+    try {
+      beforeAll(spy(trackOrder("globalBeforeAll")));
+      beforeAll(globalBeforeAll);
+      afterAll(globalAfterAll);
+      afterAll(spy(trackOrder("globalAfterAll")));
+      beforeEach(spy(trackOrder("globalBeforeEach")));
+      beforeEach(globalBeforeEach);
+      afterEach(globalAfterEach);
+      afterEach(spy(trackOrder("globalAfterEach")));
+
+      describe("d", () => {
+        beforeAll(spy(trackOrder("localBeforeAll")));
+        beforeAll(localBeforeAll);
+        afterAll(localAfterAll);
+        afterAll(spy(trackOrder("localAfterAll")));
+        beforeEach(spy(trackOrder("localBeforeEach")));
+        beforeEach(localBeforeEach);
+        afterEach(localAfterEach);
+        afterEach(spy(trackOrder("localAfterEach")));
+
+        it({ name: "t1", fn: fns[0] });
+        it({ name: "t2", fn: fns[1] });
+      });
+
+      assertSpyCalls(test, 1);
+      const options = test.calls[0]?.args[0] as Deno.TestDefinition;
+      assertEquals(options.name, "d");
+
+      await options.fn(context);
+    } finally {
+      TestSuiteInternal.reset();
+    }
+
+    // Global hooks wrap local hooks around the tests.
+    assertEquals(order, [
+      "globalBeforeAll",
+      "localBeforeAll",
+      "globalBeforeEach",
+      "localBeforeEach",
+      "localAfterEach",
+      "globalAfterEach",
+      "globalBeforeEach",
+      "localBeforeEach",
+      "localAfterEach",
+      "globalAfterEach",
+      "localAfterAll",
+      "globalAfterAll",
+    ]);
+
+    assertSpyCalls(globalBeforeAll, 1);
+    assertSpyCalls(globalAfterAll, 1);
+    assertSpyCalls(globalBeforeEach, 2);
+    assertSpyCalls(globalAfterEach, 2);
+    assertSpyCalls(localBeforeAll, 1);
+    assertSpyCalls(localAfterAll, 1);
+    assertSpyCalls(localBeforeEach, 2);
+    assertSpyCalls(localAfterEach, 2);
+    assertSpyCalls(fns[0], 1);
+    assertSpyCalls(fns[1], 1);
+  },
+);
+
+Deno.test(
+  "top-level beforeAll() with multiple nested describes registers each describe as its own Deno.test",
+  () => {
+    using test = stub(Deno, "test");
+    const beforeAllFn = spy();
+    try {
+      beforeAll(beforeAllFn);
+
+      describe("d1", () => {
+        it({ name: "t1", fn: () => {} });
+      });
+      describe("d2", () => {
+        it({ name: "t2", fn: () => {} });
+      });
+
+      assertSpyCalls(test, 2);
+      const first = test.calls[0]?.args[0] as Deno.TestDefinition;
+      const second = test.calls[1]?.args[0] as Deno.TestDefinition;
+      assertEquals(first.name, "d1");
+      assertEquals(second.name, "d2");
+    } finally {
+      TestSuiteInternal.reset();
+    }
+  },
+);
+
 Deno.test("beforeAll() with it.only() propagates only to Deno.test", () => {
   using test = stub(Deno, "test");
   try {