Skip to content

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

Open
fibibot wants to merge 1 commit into
mainfrom
orch/issue-68
Open

fix(testing): top-level hook outside describe no longer inflates step count#7138
fibibot wants to merge 1 commit into
mainfrom
orch/issue-68

Conversation

@fibibot
Copy link
Copy Markdown

@fibibot fibibot commented May 14, 2026

Summary

A top-level beforeAll/afterAll/beforeEach/afterEach call creates a synthetic "global" TestSuite to host the hook. Previously that suite was also eagerly registered as a top-level Deno.test, so any nested describe was reported as a child step of global — inflating the step count by one and changing the test output.

For example, this file:

import { assert } from "@std/assert";
import { beforeAll, describe, test } from "@std/testing/bdd";

beforeAll(() => {});

describe("the describe", () => {
  test("test 1", () => assert(Math.random() < 0.0001));
  test("test 2", () => assert(Math.random() === 0.123456));
});

reported FAILED | 0 passed | 1 failed (3 steps) even though only two tests existed. With this PR it reports (2 steps), matching the file's contents.

How the fix works

  • The synthetic "global" suite is now only registered with Deno.test lazily — only if a top-level it() is actually added to it. Otherwise the wrapper never surfaces as a counted test/step.
  • Each child describe of the synthetic suite is promoted to its own top-level Deno.test. It keeps a back-reference to the synthetic suite (syntheticParent) so the global hooks still wrap its tests at run time:
    • beforeAll / afterAll are invoked around the promoted child's body.
    • The synthetic suite's symbol is pushed onto TestSuiteInternal.active for the duration of the run, so runTest's normal traversal picks up beforeEach / afterEach from the global suite just like any other parent.
  • Top-level it() calls under a synthetic suite still work exactly as before: the synthetic suite is registered lazily on the first it() and the it()s become its steps.

Behavior preserved

  • Hook-only top-level + nested describe → fixed: nested describes register as their own Deno.tests, hooks still run around their tests (regression test added).
  • Hook + top-level it() → unchanged: a single "global" Deno.test wraps the it()s (existing tests still pass).
  • it.only() propagation to the synthetic global → unchanged (existing test still passes).
  • Mixed (hook + top-level it() + nested describe) → describes are promoted; the synthetic wrapper remains for the top-level it()s. Hooks fire around both, which means beforeAll runs once per top-level Deno.test rather than strictly once across the file. Acceptance criteria only requires the hook still "runs around tests" in this case.

Tests

Added three regression tests in testing/bdd_test.ts:

  1. Top-level hooks + only a nested describe → one Deno.test named after the user's describe, two steps, hooks fire correct number of times.
  2. Same, but the nested describe also has its own hooks → verifies the global/local hook ordering around each test.
  3. Top-level hook + multiple nested describes → each describe is its own Deno.test.

Closes bartlomieju/orchid-inbox#68

Test plan

  • Reproduction from the source issue prints (2 steps) instead of (3 steps).
  • deno test -A --parallel --no-check testing/ — 137 passed (159 steps), 0 failed.
  • deno test -A --parallel --no-check expect/ — 167 passed, 0 failed (expect builds on bdd).
  • deno fmt --check — clean.
  • deno lint — clean.
  • deno test --doc testing/bdd.ts — doc examples still pass.

…ep count

A top-level `beforeAll`/`afterAll`/`beforeEach`/`afterEach` call creates a
synthetic "global" `TestSuite` to host the hook. Previously that suite was
also eagerly registered as a top-level `Deno.test`, so any nested `describe`
would be reported as a child step of `global` — inflating the step count by
one and changing the test output.

Now the synthetic suite is only registered with `Deno.test` if a top-level
`it()` is actually added to it. Otherwise, each child `describe` is promoted
to its own `Deno.test` and inherits the global hooks at run time (so
`beforeAll`/`afterAll` wrap the nested tests, and the `active` stack picks
up the global `beforeEach`/`afterEach`).

Fixes #7056
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

❌ Patch coverage is 98.94737% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.61%. Comparing base (6510612) to head (ad8ba5f).

Files with missing lines Patch % Lines
testing/_test_suite.ts 98.93% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7138   +/-   ##
=======================================
  Coverage   94.61%   94.61%           
=======================================
  Files         634      634           
  Lines       51822    51866   +44     
  Branches     9336     9348   +12     
=======================================
+ Hits        49029    49072   +43     
  Misses       2218     2218           
- Partials      575      576    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants