Skip to content

console: match Node.js output for timeEnd/timeLog (stdout, "label: duration", 1000 ms threshold) - #34240

Closed
robobun wants to merge 2 commits into
mainfrom
claude/farm/3fc048a8/console-time-node-compat
Closed

console: match Node.js output for timeEnd/timeLog (stdout, "label: duration", 1000 ms threshold)#34240
robobun wants to merge 2 commits into
mainfrom
claude/farm/3fc048a8/console-time-node-compat

console: match Node.js output for timeEnd/timeLog

ead1fa5
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 15, 2026 in 16m 43s

Code review found 2 potential issues

Found 4 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/web/console/console-timeLog.test.ts:7 DURATION regex alternation is ungrouped — anchors/suffixes don't apply to all branches
🟡 Nit test/js/web/console/console-timeLog.test.ts:79 Busy-wait timing assertions hard-code leading '1', can flake on loaded CI

Annotations

Check warning on line 7 in test/js/web/console/console-timeLog.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

DURATION regex alternation is ungrouped — anchors/suffixes don't apply to all branches

The top-level `|` in `DURATION` is ungrouped, so when `DURATION.source` is interpolated into a larger pattern the surrounding prefix/suffix only attach to the first/last alternative. As a result the "emits exactly one trailing newline" tests match `abc: 0.1ms` with zero, one, or many trailing newlines, the `timeLog` extra-arg check passes even without ` x`, and `normalize` produces `: : <time>` for a seconds-range duration. Wrap the alternation:

```ts
const DURATION = /(?:[\d.]+ms|[\d.]+s|[\d:.

Check warning on line 79 in test/js/web/console/console-timeLog.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Busy-wait timing assertions hard-code leading '1', can flake on loaded CI

nit: pinning the leading `1` here (and in `/^d: 1\.\d{3}s\n$/` at line 111) isn't load-bearing — the property under test is "unit is `s`, not `ms`". These two busy-waits run under `it.concurrent` alongside ~7 other subprocess spawns, so on a loaded CI runner the child can be descheduled long enough for elapsed to tick past 2000 ms and print `2.xxxs`. `\d+\.\d{3}s` asserts the same thing without the flake window.