Skip to content

Add debug logging for test execution with BUN_DEBUG_jest=1#23796

Merged
Jarred-Sumner merged 4 commits into
mainfrom
claude/add-jest-test-start-logging
Oct 20, 2025
Merged

Add debug logging for test execution with BUN_DEBUG_jest=1#23796
Jarred-Sumner merged 4 commits into
mainfrom
claude/add-jest-test-start-logging

Conversation

@robobun

@robobun robobun commented Oct 18, 2025

Copy link
Copy Markdown
Collaborator

Summary

Adds debug logging that prints the name of each test when it starts running, controlled by the BUN_DEBUG_jest=1 environment variable.

Changes

  • Modified src/bun.js/test/Execution.zig to add logging in the onEntryStarted() function
  • Added a scoped logger using bun.Output.scoped(.jest, .visible)
  • When BUN_DEBUG_jest=1 is set, prints: [jest] Running test: <test name>

Testing

Manually tested with various test files:

Without BUN_DEBUG_jest:

$ bun bd test /tmp/test-jest-log.test.ts
bun test v1.3.1 (642d04b9)

 3 pass
 0 fail
 3 expect() calls
Ran 3 tests across 1 file. [2.90s]

With BUN_DEBUG_jest=1:

$ BUN_DEBUG_jest=1 bun bd test /tmp/test-jest-log.test.ts
bun test v1.3.1 (642d04b9)
[jest] Running test: first test
[jest] Running test: second test
[jest] Running test: third test

 3 pass
 0 fail
 3 expect() calls
Ran 3 tests across 1 file. [2.77s]

Also tested with nested describe blocks and all test names are logged correctly.

Notes

  • This feature is only available in debug builds (not release builds)
  • No tests were added as this is a debug-only feature
  • Helps with debugging test execution flow and understanding when tests start running

@robobun

robobun commented Oct 18, 2025

Copy link
Copy Markdown
Collaborator Author

This adds a debug log that prints the name of each test when the execution
sequence starts (before beforeEach hooks run). The log is controlled by the
BUN_DEBUG_jest=1 environment variable.

When enabled, it prints: [jest] Running test: <test name>

If a test has no name, it prints: [jest] Running test: (unnamed)

This helps with debugging test execution flow and is only available in debug builds.
@robobun robobun force-pushed the claude/add-jest-test-start-logging branch from bc6f776 to 27029e0 Compare October 18, 2025 01:21
@pfgithub pfgithub marked this pull request as ready for review October 18, 2025 02:00
@coderabbitai

coderabbitai Bot commented Oct 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adds logging functionality to the Jest test runner by introducing a new log statement in onSequenceStarted to print running test names and defines a top-level log constant for scoped Jest output visibility.

Changes

Cohort / File(s) Change Summary
Jest test execution logging
src/bun.js/test/Execution.zig
Adds at-sequence log statement to print test name when a sequence has a test entry; introduces top-level log constant for Jest-scoped visible output; formats test names with escape sequences

Possibly related PRs

  • oven-sh/bun#22896: Both PRs modify onSequenceStarted in the same file; the related PR adds early-return guards for skipped tests while this one adds logging output.
  • oven-sh/bun#22919: Both PRs modify src/bun.js/test/Execution.zig and touch related test-runner output paths.

Suggested reviewers

  • lydiahallie

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description provides comprehensive information about what the PR does and how it was tested, including detailed manual testing examples with and without the debug flag. However, the description does not follow the required template structure specified for this repository. The template requires specific section headings "### What does this PR do?" and "### How did you verify your code works?" but the provided description uses different headings (Summary, Changes, Testing, Notes) instead. While the content itself is present and well-written, the structural template requirements are not met.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Add debug logging for test execution with BUN_DEBUG_jest=1" clearly and concisely summarizes the main change in the changeset. It accurately describes the core functionality being added: debug logging controlled by the BUN_DEBUG_jest environment variable. The title is specific enough that a teammate reviewing the history would understand the primary change without being vague or overly broad. The content of the pull request (adding log statements to print test names) directly aligns with this title.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d9a867a and efa8109.

📒 Files selected for processing (1)
  • src/bun.js/test/Execution.zig (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue

Files:

  • src/bun.js/test/Execution.zig
src/bun.js/**/*.zig

📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)

src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy

Files:

  • src/bun.js/test/Execution.zig
src/**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, manage memory carefully and use defer for cleanup of allocations/resources

When adding debug logs in Zig, create a scoped logger and log via Bun APIs: const log = bun.Output.scoped(.${SCOPE}, .hidden); then log("...", .{})

src/**/*.zig: Use private fields in Zig with the # prefix (e.g., struct { #foo: u32 };)
Prefer decl literals in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing @import statements at the bottom of the Zig file (formatter may reorder automatically)
Prefer @import("bun") rather than @import("root").bun or @import("../bun.zig")

Files:

  • src/bun.js/test/Execution.zig
🧠 Learnings (1)
📚 Learning: 2025-10-08T03:11:45.286Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-10-08T03:11:45.286Z
Learning: Applies to src/**/*.zig : When adding debug logs in Zig, create a scoped logger and log via Bun APIs: `const log = bun.Output.scoped(.${SCOPE}, .hidden);` then `log("...", .{})`

Applied to files:

  • src/bun.js/test/Execution.zig
🔇 Additional comments (1)
src/bun.js/test/Execution.zig (1)

497-498: Log implementation looks good, pending visibility fix.

The log call correctly:

  • Uses std.zig.fmtEscapes to escape the test name
  • Handles unnamed tests with a fallback
  • Is guarded by appropriate conditions (test_entry exists and has a callback)
  • Provides a clear, descriptive message

Note: The effectiveness of this logging depends on the visibility level being correctly set to .hidden in the log constant declaration (see previous comment).

Comment on lines +624 to +625
const log = bun.Output.scoped(.jest, .visible);

@coderabbitai coderabbitai Bot Oct 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the visibility level: should this be .hidden instead of .visible?

According to the coding guideline and the PR description, debug logs should use .hidden to gate them behind the BUN_DEBUG_jest=1 environment variable. The current implementation uses .visible, which may cause these logs to always appear rather than only when the environment variable is set.

Based on learnings

Apply this diff if .hidden is the correct visibility level:

-const log = bun.Output.scoped(.jest, .visible);
+const log = bun.Output.scoped(.jest, .hidden);

Alternatively, please confirm whether .visible is intentional and explain how the BUN_DEBUG_jest=1 environment variable gates the output with this visibility setting.


I need to investigate the codebase to verify the correct visibility level for the logger.


---

🏁 Script executed:

```shell
#!/bin/bash
# Find examples of bun.Output.scoped usage in Zig files to identify the correct pattern
rg -n 'bun\.Output\.scoped' --type zig -B2 -A2

Length of output: 39967


🏁 Script executed:

#!/bin/bash
# Search for BUN_DEBUG environment variable usage to understand gating behavior
rg -n 'BUN_DEBUG' --type zig -B3 -A3 | head -100

Length of output: 6820


🏁 Script executed:

#!/bin/bash
# Check if there's documentation or configuration for visibility levels
rg -n '\.hidden|\.visible' --type zig | grep -i output | head -50

Length of output: 4349


🏁 Script executed:

#!/bin/bash
# Look at the context around line 624 in the file under review to confirm the scope
sed -n '620,630p' src/bun.js/test/Execution.zig

Length of output: 207


🏁 Script executed:

#!/bin/bash
# Search for other loggers declared in Execution.zig or nearby test files
rg -n 'const log = ' src/bun.js/test/ --type zig -B1 -A1

Length of output: 151


Change .visible to .hidden to enable environment variable gating.

The logger at line 624 currently uses .visible, which makes logs always appear. However, the PR states logging should be "enabled by the environment variable BUN_DEBUG_jest=1". According to src/output.zig (lines 767-773), this behavior requires .hidden visibility. Additionally, src/cli/test/Scanner.zig:18 already declares a jest-scoped logger with .hidden, confirming the correct pattern for this module.

-const log = bun.Output.scoped(.jest, .visible);
+const log = bun.Output.scoped(.jest, .hidden);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const log = bun.Output.scoped(.jest, .visible);
const log = bun.Output.scoped(.jest, .hidden);
🤖 Prompt for AI Agents
In src/bun.js/test/Execution.zig around lines 624-625, the logger is created
with bun.Output.scoped(.jest, .visible) which forces logs to always appear;
change the visibility argument from .visible to .hidden so the jest-scoped
logger respects the environment gating (BUN_DEBUG_jest=1) as implemented in
src/output.zig and consistent with src/cli/test/Scanner.zig: use
bun.Output.scoped(.jest, .hidden) instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Visible means it is enabled by default in debug builds unless BUN_DEBUG_QUIET_LOGS=1 is set, which is what we want. If that is set, then it is only enabled with BUN_DEBUG_jest=1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfgithub Thank you for the clarification! I understand now—.visible is indeed the correct choice here. It enables the logs by default in debug builds, while still respecting BUN_DEBUG_QUIET_LOGS=1 to silence them, and BUN_DEBUG_jest=1 to re-enable them when quieted.

I appreciate you taking the time to explain the distinction between .visible and .hidden for scoped loggers. This is helpful context for understanding the debug logging system.

    /)_/)
   (o.o)  oops, my mistake!
   (   )

✏️ Learnings added
Learnt from: pfgithub
PR: oven-sh/bun#23796
File: src/bun.js/test/Execution.zig:624-625
Timestamp: 2025-10-18T02:06:31.595Z
Learning: In Zig files using bun.Output.scoped(), the visibility level `.visible` means logs are enabled by default in debug builds unless `BUN_DEBUG_QUIET_LOGS=1` is set; if that environment variable is set, the logs can be re-enabled with `BUN_DEBUG_<scope>=1`. Use `.visible` for logs that should appear by default in debug builds, and `.hidden` for logs that require explicit opt-in via `BUN_DEBUG_<scope>=1`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-10-08T03:11:45.286Z
Learning: Applies to src/**/*.zig : When adding debug logs in Zig, create a scoped logger and log via Bun APIs: `const log = bun.Output.scoped(.${SCOPE}, .hidden);` then `log("...", .{})`

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions

Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)

Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Use BUN_DEBUG_QUIET_LOGS=1 to silence debug logs or BUN_DEBUG_<scope>=1 to enable scoped logs in debug builds

@Jarred-Sumner Jarred-Sumner merged commit e63a897 into main Oct 20, 2025
63 of 66 checks passed
@Jarred-Sumner Jarred-Sumner deleted the claude/add-jest-test-start-logging branch October 20, 2025 04:32
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.

3 participants