bun test: nest results under describe scopes instead of repeating the scope path - #35793
bun test: nest results under describe scopes instead of repeating the scope path#35793jakeboone02 wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughBun’s test console reporter now renders nested ChangesNested test reporting
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/test/bun-test.test.ts`:
- Around line 1526-1537: Update the run helper to drain proc.stdout alongside
proc.stderr, such as by reading both streams in the existing Promise.all, while
preserving the current stderr normalization and exitCode return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51767a38-8693-4d0e-b345-c55fd0232c3f
📒 Files selected for processing (17)
docs/guides/test/coverage-threshold.mdxdocs/guides/test/coverage.mdxdocs/test/reporters.mdxsrc/runtime/cli/test/parallel/Coordinator.rssrc/runtime/cli/test/parallel/Frame.rssrc/runtime/cli/test/parallel/runner.rssrc/runtime/cli/test_command.rssrc/runtime/test_runner/bun_test.rstest/cli/test/bun-test.test.tstest/cli/test/test-filter-lifecycle-snapshot.test.tstest/js/bun/test/bun_test.test.tstest/js/bun/test/concurrent.test.tstest/js/bun/test/describe.test.tstest/js/bun/test/nested-describes.test.tstest/regression/issue/12782.test.tstest/regression/issue/19875.test.tstest/regression/issue/20092.test.ts
What does this PR do?
bun testrepeats the full describe path on every single result line. In a suite withany real nesting that means the same 40 characters of scope are re-printed hundreds of
times, and the actual test names are pushed off to the right.
This PR prints each
describescope once, as its own dim header line, and indentsits tests beneath it — two spaces per level.
Before
After
Headers are emitted lazily: a scope line is written the first time a result inside it is
about to print, and re-announced if results from a different scope interleave in between.
Nothing is buffered — every line still goes out the instant the test finishes.
Design notes & edge cases
printed_scope_path: Vec<u8>(\x1f-separated). Before printing a result it diffs thetest's scope path against the last-printed one, emits headers for the segments that
differ, and stores the new path. This is O(depth) per test with one allocation reused
for the whole run.
describe(() => {...})contributes no headerand no indent level, so the anonymous scope doesn't shift its children right.
BoundedArray), matching the existing limit incollect_scopes. Beyond that, indentation stops growing rather than running away.still read
math > arithmetic > subtract— they're a flat index, and nesting them out ofcontext would be worse. This means each line is formatted twice, so the side effects of
formatting (assertion-count errors, GitHub Actions timeout annotations) were extracted
into
print_status_side_effectsso they fire exactly once.--reporter=dotsis untouched. It usesLayout::Flatand never nests.--only-failuresnests the same way; scopes with no failures never print a header.How did you verify your code works?
Test runs
bun bd test test/cli/test/— 220 pass, 6 todo, 0 failbun bd test test/js/bun/test/— no regressions vs. the parent commit(
df6c7eed6bhas 6 pre-existing failures, 4 of them missing-node_modulesloaderrors; this branch has the same set)
test/regression/issue/*.test.tsfiles that spawnbun test— green(5 inline snapshots regenerated for the new shape)
New tests in
test/cli/test/bun-test.test.tscover indentation depth, unnamed-describetransparency, re-announcing a scope after interleaving, and
--only-failures. They failunder
USE_SYSTEM_BUN=1and pass underbun bd test.Manually verified: default reporter,
FORCE_COLOR=1,NO_COLOR=1,--parallel=2,--reporter=dots,--only-failures,--rerun-each=2,test.concurrent,--bail,process.exit()mid-file, GitHub Actions::group::output, and JUnit output.Performance
Release builds, macOS arm64, best-of-5, stdout discarded, stderr piped.
Workloads: wide = 200 files x 200 tests x 3 levels (40k tests); deep = 20 files x
5,000 tests x 8 levels (100k tests).
Full benchmark matrix (all 3 builds x 2 workloads x 4 modes)
baseline=df6c7eed6b(main),streaming= this branch,buffered= the sibling PR.Summary: memory is unchanged (single reused
Vec<u8>for the scope path, ~1 KB).Wall time is +3% on
wideand +11% ondeepfor the default reporter;--dotsand--only-failuresare within noise. Time-to-first-output is identical to today.Streaming vs. buffered
test.concurrent/--parallelgroupingdescribelines (#12378)src/Streaming is the conservative option: it changes only how the scope prefix is rendered and
keeps every other timing property of the reporter identical. The cost is that under
test.concurrentor--paralleladescribecan appear more than once, because thereporter can't know what's still coming.
Known limitations
test.concurrentthis is common; with sequential tests it never happens.a > b > nameform (intentional, see designnotes).