mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix(debugger): retroactively report tests when TestReporter.enable is called (#25986)
## Summary - Fixes #25972: TestReporter domain events not firing when debugger connects after test discovery When a debugger client connects and enables the TestReporter domain after tests have been discovered (e.g., using `--inspect` instead of `--inspect-wait`), the `TestReporter.found`, `TestReporter.start`, and `TestReporter.end` events would not fire. This is because tests discovered without an enabled debugger have `test_id_for_debugger = 0`, and the event emission code checks for non-zero IDs. The fix retroactively assigns test IDs and reports discovered tests when `TestReporter.enable` is called: 1. Check if there's an active test file in collection or execution phase 2. Iterate through the test tree (DescribeScopes and test entries) 3. Assign unique `test_id_for_debugger` values to each test/describe 4. Send `TestReporter.found` events for each discovered test ## Test plan - [ ] Verify IDE integrations can now receive test telemetry when connecting after test discovery - [ ] Ensure existing `--inspect-wait` behavior continues to work (debugger enabled before discovery) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -316,6 +316,7 @@ pub const TestReporterAgent = struct {
|
||||
|
||||
pub const Handle = opaque {
|
||||
extern "c" fn Bun__TestReporterAgentReportTestFound(agent: *Handle, callFrame: *jsc.CallFrame, testId: c_int, name: *bun.String, item_type: TestType, parentId: c_int) void;
|
||||
extern "c" fn Bun__TestReporterAgentReportTestFoundWithLocation(agent: *Handle, testId: c_int, name: *bun.String, item_type: TestType, parentId: c_int, sourceURL: *bun.String, line: c_int) void;
|
||||
extern "c" fn Bun__TestReporterAgentReportTestStart(agent: *Handle, testId: c_int) void;
|
||||
extern "c" fn Bun__TestReporterAgentReportTestEnd(agent: *Handle, testId: c_int, bunTestStatus: TestStatus, elapsed: f64) void;
|
||||
|
||||
@@ -323,6 +324,10 @@ pub const TestReporterAgent = struct {
|
||||
Bun__TestReporterAgentReportTestFound(this, callFrame, testId, name, item_type, parentId);
|
||||
}
|
||||
|
||||
pub fn reportTestFoundWithLocation(this: *Handle, testId: i32, name: *bun.String, item_type: TestType, parentId: i32, sourceURL: *bun.String, line: i32) void {
|
||||
Bun__TestReporterAgentReportTestFoundWithLocation(this, testId, name, item_type, parentId, sourceURL, line);
|
||||
}
|
||||
|
||||
pub fn reportTestStart(this: *Handle, testId: c_int) void {
|
||||
Bun__TestReporterAgentReportTestStart(this, testId);
|
||||
}
|
||||
@@ -335,8 +340,88 @@ pub const TestReporterAgent = struct {
|
||||
if (VirtualMachine.get().debugger) |*debugger| {
|
||||
debug("enable", .{});
|
||||
debugger.test_reporter_agent.handle = agent;
|
||||
|
||||
// Retroactively report any tests that were already discovered before the debugger connected
|
||||
retroactivelyReportDiscoveredTests(agent);
|
||||
}
|
||||
}
|
||||
|
||||
/// When TestReporter.enable is called after test collection has started/finished,
|
||||
/// we need to retroactively assign test IDs and report discovered tests.
|
||||
fn retroactivelyReportDiscoveredTests(agent: *Handle) void {
|
||||
const Jest = jsc.Jest.Jest;
|
||||
const runner = Jest.runner orelse return;
|
||||
const active_file = runner.bun_test_root.active_file.get() orelse return;
|
||||
|
||||
// Only report if we're in collection or execution phase (tests have been discovered)
|
||||
switch (active_file.phase) {
|
||||
.collection, .execution => {},
|
||||
.done => return,
|
||||
}
|
||||
|
||||
// Get the file path for source location info
|
||||
const file_path = runner.files.get(active_file.file_id).source.path.text;
|
||||
var source_url = bun.String.init(file_path);
|
||||
|
||||
// Track the maximum ID we assign
|
||||
var max_id: i32 = 0;
|
||||
|
||||
// Recursively report all discovered tests starting from root scope
|
||||
const root_scope = active_file.collection.root_scope;
|
||||
retroactivelyReportScope(agent, root_scope, -1, &max_id, &source_url);
|
||||
|
||||
debug("retroactively reported {} tests", .{max_id});
|
||||
}
|
||||
|
||||
fn retroactivelyReportScope(agent: *Handle, scope: *bun_test.DescribeScope, parent_id: i32, max_id: *i32, source_url: *bun.String) void {
|
||||
for (scope.entries.items) |*entry| {
|
||||
switch (entry.*) {
|
||||
.describe => |describe| {
|
||||
// Only report and assign ID if not already assigned
|
||||
if (describe.base.test_id_for_debugger == 0) {
|
||||
max_id.* += 1;
|
||||
const test_id = max_id.*;
|
||||
// Assign the ID so start/end events will fire during execution
|
||||
describe.base.test_id_for_debugger = test_id;
|
||||
var name = bun.String.init(describe.base.name orelse "(unnamed)");
|
||||
agent.reportTestFoundWithLocation(
|
||||
test_id,
|
||||
&name,
|
||||
.describe,
|
||||
parent_id,
|
||||
source_url,
|
||||
@intCast(describe.base.line_no),
|
||||
);
|
||||
// Recursively report children with this describe as parent
|
||||
retroactivelyReportScope(agent, describe, test_id, max_id, source_url);
|
||||
} else {
|
||||
// Already has ID, just recurse with existing ID as parent
|
||||
retroactivelyReportScope(agent, describe, describe.base.test_id_for_debugger, max_id, source_url);
|
||||
}
|
||||
},
|
||||
.test_callback => |test_entry| {
|
||||
// Only report and assign ID if not already assigned
|
||||
if (test_entry.base.test_id_for_debugger == 0) {
|
||||
max_id.* += 1;
|
||||
const test_id = max_id.*;
|
||||
// Assign the ID so start/end events will fire during execution
|
||||
test_entry.base.test_id_for_debugger = test_id;
|
||||
var name = bun.String.init(test_entry.base.name orelse "(unnamed)");
|
||||
agent.reportTestFoundWithLocation(
|
||||
test_id,
|
||||
&name,
|
||||
.@"test",
|
||||
parent_id,
|
||||
source_url,
|
||||
@intCast(test_entry.base.line_no),
|
||||
);
|
||||
}
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const bun_test = jsc.Jest.bun_test;
|
||||
pub export fn Bun__TestReporterAgentDisable(_: *Handle) void {
|
||||
if (VirtualMachine.get().debugger) |*debugger| {
|
||||
debug("disable", .{});
|
||||
|
||||
@@ -50,6 +50,26 @@ void Bun__TestReporterAgentReportTestFound(Inspector::InspectorTestReporterAgent
|
||||
agent->reportTestFound(callFrame, testId, str, type, parentId);
|
||||
}
|
||||
|
||||
void Bun__TestReporterAgentReportTestFoundWithLocation(Inspector::InspectorTestReporterAgent* agent, int testId, BunString* name, BunTestType item_type, int parentId, BunString* sourceURL, int line)
|
||||
{
|
||||
auto str = name->toWTFString(BunString::ZeroCopy);
|
||||
auto sourceURLStr = sourceURL->toWTFString(BunString::ZeroCopy);
|
||||
|
||||
Protocol::TestReporter::TestType type;
|
||||
switch (item_type) {
|
||||
case BunTestType::Test:
|
||||
type = Protocol::TestReporter::TestType::Test;
|
||||
break;
|
||||
case BunTestType::Describe:
|
||||
type = Protocol::TestReporter::TestType::Describe;
|
||||
break;
|
||||
default:
|
||||
ASSERT_NOT_REACHED();
|
||||
}
|
||||
|
||||
agent->reportTestFoundWithLocation(testId, str, type, parentId, sourceURLStr, line);
|
||||
}
|
||||
|
||||
void Bun__TestReporterAgentReportTestStart(Inspector::InspectorTestReporterAgent* agent, int testId)
|
||||
{
|
||||
agent->reportTestStart(testId);
|
||||
@@ -211,6 +231,21 @@ void InspectorTestReporterAgent::reportTestFound(JSC::CallFrame* callFrame, int
|
||||
parentId > 0 ? parentId : std::optional<int>());
|
||||
}
|
||||
|
||||
void InspectorTestReporterAgent::reportTestFoundWithLocation(int testId, const String& name, Protocol::TestReporter::TestType type, int parentId, const String& sourceURL, int line)
|
||||
{
|
||||
if (!m_enabled)
|
||||
return;
|
||||
|
||||
m_frontendDispatcher->found(
|
||||
testId,
|
||||
String(), // sourceID - not available for retroactively reported tests
|
||||
sourceURL,
|
||||
line,
|
||||
name,
|
||||
type,
|
||||
parentId > 0 ? parentId : std::optional<int>());
|
||||
}
|
||||
|
||||
void InspectorTestReporterAgent::reportTestStart(int testId)
|
||||
{
|
||||
if (!m_enabled || !m_frontendDispatcher)
|
||||
|
||||
@@ -34,6 +34,7 @@ public:
|
||||
|
||||
// Public API for reporting test events
|
||||
void reportTestFound(JSC::CallFrame*, int testId, const String& name, Protocol::TestReporter::TestType type = Protocol::TestReporter::TestType::Test, int parentId = -1);
|
||||
void reportTestFoundWithLocation(int testId, const String& name, Protocol::TestReporter::TestType type, int parentId, const String& sourceURL, int line);
|
||||
void reportTestStart(int testId);
|
||||
void reportTestEnd(int testId, Protocol::TestReporter::TestStatus status, double elapsed);
|
||||
|
||||
|
||||
262
test/cli/inspect/test-reporter.test.ts
Normal file
262
test/cli/inspect/test-reporter.test.ts
Normal file
@@ -0,0 +1,262 @@
|
||||
import { Subprocess, spawn } from "bun";
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { bunEnv, bunExe, isPosix, tempDir } from "harness";
|
||||
import { join } from "node:path";
|
||||
import { InspectorSession, connect } from "./junit-reporter";
|
||||
import { SocketFramer } from "./socket-framer";
|
||||
|
||||
/**
|
||||
* Extended InspectorSession with helper methods for TestReporter testing
|
||||
*/
|
||||
class TestReporterSession extends InspectorSession {
|
||||
private foundTests: Map<number, any> = new Map();
|
||||
private startedTests: Set<number> = new Set();
|
||||
private endedTests: Map<number, any> = new Map();
|
||||
|
||||
constructor() {
|
||||
super();
|
||||
this.setupTestEventListeners();
|
||||
}
|
||||
|
||||
private setupTestEventListeners() {
|
||||
this.addEventListener("TestReporter.found", (params: any) => {
|
||||
this.foundTests.set(params.id, params);
|
||||
});
|
||||
this.addEventListener("TestReporter.start", (params: any) => {
|
||||
this.startedTests.add(params.id);
|
||||
});
|
||||
this.addEventListener("TestReporter.end", (params: any) => {
|
||||
this.endedTests.set(params.id, params);
|
||||
});
|
||||
}
|
||||
|
||||
enableInspector() {
|
||||
this.send("Inspector.enable");
|
||||
}
|
||||
|
||||
enableTestReporter() {
|
||||
this.send("TestReporter.enable");
|
||||
}
|
||||
|
||||
enableAll() {
|
||||
this.send("Inspector.enable");
|
||||
this.send("TestReporter.enable");
|
||||
this.send("LifecycleReporter.enable");
|
||||
this.send("Console.enable");
|
||||
this.send("Runtime.enable");
|
||||
}
|
||||
|
||||
initialize() {
|
||||
this.send("Inspector.initialized");
|
||||
}
|
||||
|
||||
unref() {
|
||||
this.socket?.unref();
|
||||
}
|
||||
|
||||
ref() {
|
||||
this.socket?.ref();
|
||||
}
|
||||
|
||||
getFoundTests() {
|
||||
return this.foundTests;
|
||||
}
|
||||
|
||||
getStartedTests() {
|
||||
return this.startedTests;
|
||||
}
|
||||
|
||||
getEndedTests() {
|
||||
return this.endedTests;
|
||||
}
|
||||
|
||||
clearFoundTests() {
|
||||
this.foundTests.clear();
|
||||
}
|
||||
|
||||
waitForEvent(eventName: string, timeout = 10000): Promise<any> {
|
||||
this.ref();
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
reject(new Error(`Timeout waiting for event: ${eventName}`));
|
||||
}, timeout);
|
||||
|
||||
const listener = (params: any) => {
|
||||
clearTimeout(timer);
|
||||
resolve(params);
|
||||
};
|
||||
|
||||
this.addEventListener(eventName, listener);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Wait for a specific number of TestReporter.found events
|
||||
*/
|
||||
waitForFoundTests(count: number, timeout = 10000): Promise<Map<number, any>> {
|
||||
this.ref();
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
reject(
|
||||
new Error(
|
||||
`Timeout waiting for ${count} found tests, got ${this.foundTests.size}: ${JSON.stringify([...this.foundTests.values()])}`,
|
||||
),
|
||||
);
|
||||
}, timeout);
|
||||
|
||||
const check = () => {
|
||||
if (this.foundTests.size >= count) {
|
||||
clearTimeout(timer);
|
||||
resolve(this.foundTests);
|
||||
}
|
||||
};
|
||||
|
||||
// Check immediately in case we already have enough
|
||||
check();
|
||||
|
||||
// Also listen for new events
|
||||
this.addEventListener("TestReporter.found", check);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Wait for a specific number of TestReporter.end events
|
||||
*/
|
||||
waitForEndedTests(count: number, timeout = 10000): Promise<Map<number, any>> {
|
||||
this.ref();
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
reject(new Error(`Timeout waiting for ${count} ended tests, got ${this.endedTests.size}`));
|
||||
}, timeout);
|
||||
|
||||
const check = () => {
|
||||
if (this.endedTests.size >= count) {
|
||||
clearTimeout(timer);
|
||||
resolve(this.endedTests);
|
||||
}
|
||||
};
|
||||
|
||||
check();
|
||||
this.addEventListener("TestReporter.end", check);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
describe.if(isPosix)("TestReporter inspector protocol", () => {
|
||||
let proc: Subprocess | undefined;
|
||||
let socket: ReturnType<typeof connect> extends Promise<infer T> ? T : never;
|
||||
|
||||
afterEach(() => {
|
||||
proc?.kill();
|
||||
proc = undefined;
|
||||
// @ts-ignore - close the socket if it exists
|
||||
socket?.end?.();
|
||||
socket = undefined as any;
|
||||
});
|
||||
|
||||
test("retroactively reports tests when TestReporter.enable is called after tests are discovered", async () => {
|
||||
// This test specifically verifies that when TestReporter.enable is called AFTER
|
||||
// test collection has started, the already-discovered tests are retroactively reported.
|
||||
//
|
||||
// The flow is:
|
||||
// 1. Connect to inspector and enable only Inspector domain (NOT TestReporter)
|
||||
// 2. Send Inspector.initialized to allow test collection and execution to proceed
|
||||
// 3. Wait briefly for test collection to complete
|
||||
// 4. THEN send TestReporter.enable - this should trigger retroactive reporting
|
||||
// of tests that were discovered but not yet reported
|
||||
|
||||
using dir = tempDir("test-reporter-delayed-enable", {
|
||||
"delayed.test.ts": `
|
||||
import { describe, test, expect } from "bun:test";
|
||||
|
||||
describe("suite A", () => {
|
||||
test("test A1", async () => {
|
||||
// Add delay to ensure we have time to enable TestReporter during execution
|
||||
await Bun.sleep(500);
|
||||
expect(1).toBe(1);
|
||||
});
|
||||
test("test A2", () => {
|
||||
expect(2).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("suite B", () => {
|
||||
test("test B1", () => {
|
||||
expect(3).toBe(3);
|
||||
});
|
||||
});
|
||||
`,
|
||||
});
|
||||
|
||||
const socketPath = join(String(dir), `inspector-${Math.random().toString(36).substring(2)}.sock`);
|
||||
|
||||
const session = new TestReporterSession();
|
||||
const framer = new SocketFramer((message: string) => {
|
||||
session.onMessage(message);
|
||||
});
|
||||
|
||||
const socketPromise = connect(`unix://${socketPath}`).then(s => {
|
||||
socket = s;
|
||||
session.socket = s;
|
||||
session.framer = framer;
|
||||
s.data = {
|
||||
onData: framer.onData.bind(framer),
|
||||
};
|
||||
return s;
|
||||
});
|
||||
|
||||
proc = spawn({
|
||||
cmd: [bunExe(), `--inspect-wait=unix:${socketPath}`, "test", "delayed.test.ts"],
|
||||
env: bunEnv,
|
||||
cwd: String(dir),
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
|
||||
await socketPromise;
|
||||
|
||||
// Enable Inspector only (NOT TestReporter)
|
||||
session.enableInspector();
|
||||
|
||||
// Signal ready - this allows test collection and execution to proceed
|
||||
session.initialize();
|
||||
|
||||
// Wait for test collection and first test to start running
|
||||
// The first test has a 500ms sleep, so waiting 200ms ensures we're in execution phase
|
||||
await Bun.sleep(200);
|
||||
|
||||
// Now enable TestReporter - this should trigger retroactive reporting
|
||||
// of all tests that were discovered while TestReporter was disabled
|
||||
session.enableTestReporter();
|
||||
|
||||
// We should receive found events for all tests retroactively
|
||||
// Structure: 2 describes + 3 tests = 5 items
|
||||
const foundTests = await session.waitForFoundTests(5, 15000);
|
||||
expect(foundTests.size).toBe(5);
|
||||
|
||||
const testsArray = [...foundTests.values()];
|
||||
const describes = testsArray.filter(t => t.type === "describe");
|
||||
const tests = testsArray.filter(t => t.type === "test");
|
||||
|
||||
expect(describes.length).toBe(2);
|
||||
expect(tests.length).toBe(3);
|
||||
|
||||
// Verify the test names
|
||||
const testNames = tests.map(t => t.name).sort();
|
||||
expect(testNames).toEqual(["test A1", "test A2", "test B1"]);
|
||||
|
||||
// Verify describe names
|
||||
const describeNames = describes.map(d => d.name).sort();
|
||||
expect(describeNames).toEqual(["suite A", "suite B"]);
|
||||
|
||||
// Wait for tests to complete
|
||||
const endedTests = await session.waitForEndedTests(3, 15000);
|
||||
expect(endedTests.size).toBe(3);
|
||||
|
||||
const exitCode = await proc.exited;
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user