Skip to content

Commit 8e28f71

Browse files
committed
fix(core): address Codex review on --trace
- Skip Perfetto helper server in non-TTY runs. `rstest run` previously awaited `traceController.waitForExit()` unconditionally, which hangs CI because the signal it waits for never arrives. `finalize()` now checks `process.stdin.isTTY` and writes the file without starting the server in non-interactive contexts. - Give each PhaseTracker a unique `tid`. With `isolate: false` a worker runs multiple test files, and the previous "first testPath per pid" metadata collapsed every later file to the first one's label. Each tracker now gets a per-process incrementing tid; `buildTraceFile` emits `process_name = "worker <pid>"` once per pid and `thread_name = <test file>` once per (pid, tid), so multi-file workers render as separate thread tracks in Perfetto.
1 parent 835f41b commit 8e28f71

2 files changed

Lines changed: 55 additions & 22 deletions

File tree

packages/core/src/runtime/worker/phaseTracker.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,20 @@ type SliceCat = 'phase' | 'suite' | 'case';
4242
* - heap-usage counter samples (`ph: 'C'`, `cat: 'memory'`) at each phase
4343
* boundary, so memory pressure shows up as a track in Perfetto UI
4444
*
45-
* All events use `pid = process.pid`. The host later merges these into a
46-
* single trace JSON file.
45+
* All events use `pid = process.pid`. Each tracker (i.e. each test file)
46+
* gets its own `tid`, so when a worker is reused across multiple files
47+
* (`isolate: false`) each file shows up as its own thread track in
48+
* Perfetto instead of collapsing to the first file's label. The host
49+
* later merges every worker's events into a single trace JSON file.
4750
*/
4851
export class PhaseTracker {
4952
private currentPhase: PhaseName | null = null;
5053
private currentStart = 0;
5154
private hasData = false;
5255
private readonly totals: PhaseTimings = emptyPhaseTimings();
5356
private readonly trace: TraceState | null;
54-
/** Workers are single-threaded; pid doubles as tid for Perfetto tracks. */
5557
private readonly pid = process.pid;
58+
private readonly tid = nextThreadId++;
5659

5760
constructor(options: PhaseTrackerOptions = {}) {
5861
this.trace = options.trace
@@ -152,7 +155,7 @@ export class PhaseTracker {
152155
ts: startMs * 1000,
153156
dur: durMs * 1000,
154157
pid: this.pid,
155-
tid: this.pid,
158+
tid: this.tid,
156159
args: {
157160
testPath: this.trace.meta.testPath,
158161
project: this.trace.meta.project,
@@ -170,7 +173,7 @@ export class PhaseTracker {
170173
ph: 'C',
171174
ts: nowMs * 1000,
172175
pid: this.pid,
173-
tid: this.pid,
176+
tid: this.tid,
174177
args: {
175178
heapUsedMB: round2(mem.heapUsed / 1024 / 1024),
176179
heapTotalMB: round2(mem.heapTotal / 1024 / 1024),
@@ -181,3 +184,10 @@ export class PhaseTracker {
181184
}
182185

183186
const round2 = (n: number): number => Math.round(n * 100) / 100;
187+
188+
/**
189+
* Monotonic counter for `tid` assignment within a worker process. Resets per
190+
* process (each fork starts at 1), which is fine because Perfetto's `tid` is
191+
* only required to be unique within a `pid`.
192+
*/
193+
let nextThreadId = 1;

packages/core/src/utils/trace.ts

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,15 @@ const getTraceOutputPath = (rootPath: string): string => {
4242

4343
/**
4444
* Wrap raw phase events into the Perfetto trace JSON envelope and prepend
45-
* per-pid metadata so each worker shows up under its test file name
46-
* (relative to `rootPath`) instead of the default `Process <pid>` label.
45+
* Perfetto metadata so each (pid, tid) renders with the right label.
4746
*
48-
* Metadata emitted per pid:
49-
* - `process_name`: friendly test file path
50-
* - `process_sort_index`: order of first appearance, so the Perfetto UI
51-
* lists files roughly in execution order instead of by pid
52-
* - `thread_name`: a stable "worker" label on the main thread
47+
* - `process_name` (per pid) labels the worker as `worker <pid>` so multiple
48+
* files inside the same worker (with `isolate: false`) don't collapse to
49+
* the first file's path.
50+
* - `process_sort_index` (per pid) lists workers in the order they first
51+
* produced events, instead of the OS pid order.
52+
* - `thread_name` (per pid+tid) labels each tracker's thread track with the
53+
* test file path it ran (relative to `rootPath`).
5354
*/
5455
const buildTraceFile = (
5556
events: TraceEvent[],
@@ -70,22 +71,32 @@ const buildTraceFile = (
7071
args,
7172
});
7273

73-
const seen = new Set<number>();
74+
const seenPid = new Set<number>();
75+
const seenThread = new Set<string>();
7476
const metadata: TraceEvent[] = [];
7577
let sortIndex = 0;
7678
for (const ev of events) {
77-
if (seen.has(ev.pid)) continue;
7879
const testPath =
7980
typeof ev.args?.testPath === 'string' ? ev.args.testPath : undefined;
8081
if (!testPath) continue;
81-
seen.add(ev.pid);
82-
const rel = relative(rootPath, testPath);
83-
const display = rel && !rel.startsWith('..') ? rel : testPath;
84-
metadata.push(
85-
meta('process_name', ev.pid, ev.tid, { name: display }),
86-
meta('process_sort_index', ev.pid, ev.tid, { sort_index: sortIndex++ }),
87-
meta('thread_name', ev.pid, ev.tid, { name: 'worker' }),
88-
);
82+
83+
if (!seenPid.has(ev.pid)) {
84+
seenPid.add(ev.pid);
85+
metadata.push(
86+
meta('process_name', ev.pid, ev.tid, { name: `worker ${ev.pid}` }),
87+
meta('process_sort_index', ev.pid, ev.tid, {
88+
sort_index: sortIndex++,
89+
}),
90+
);
91+
}
92+
93+
const threadKey = `${ev.pid}:${ev.tid}`;
94+
if (!seenThread.has(threadKey)) {
95+
seenThread.add(threadKey);
96+
const rel = relative(rootPath, testPath);
97+
const display = rel && !rel.startsWith('..') ? rel : testPath;
98+
metadata.push(meta('thread_name', ev.pid, ev.tid, { name: display }));
99+
}
89100
}
90101
return { traceEvents: [...metadata, ...events] };
91102
};
@@ -268,6 +279,18 @@ export const createTraceController = (options: {
268279
color.gray(' Perfetto trace file: '),
269280
color.cyan(tracePath),
270281
);
282+
// The helper server keeps the event loop alive until SIGINT, which
283+
// would hang `rstest run` in CI. Only start it when stdin is a TTY
284+
// (i.e. interactive), otherwise leave the file for the user to
285+
// download from the CI artifact and open in Perfetto UI manually.
286+
if (!process.stdin.isTTY) {
287+
logger.log(
288+
color.gray(
289+
' Drag the file above into https://ui.perfetto.dev to view.',
290+
),
291+
);
292+
return;
293+
}
271294
if (!server) {
272295
try {
273296
server = await startTraceServer(tracePath);

0 commit comments

Comments
 (0)