Skip to content

ci: timestamp the browser lane's test server lines - #1789

Merged
devswha merged 1 commit into
mainfrom
fix/browser-lane-server-timestamps
Oct 23, 2026
Merged

devswha merged 1 commit into
mainfrom
fix/browser-lane-server-timestamps

Conversation

@devswha

@devswha devswha commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Change

The browser lane's script output is now timestamped line by line, so a line printed by a test's web UI server can be placed against the PASS lines and the job's timings.

  • Parallel runner (scripts/ci-browser-parallel.ts, what CI's two browser jobs run): every line written to a script's own log in .ci/browser-logs/ gets an ISO timestamp prefix ([2026-10-23T22:41:05.123Z] …).
    • stdout and stderr are both read line by line.
    • A trailing partial line is kept and written at exit.
    • failureSite() strips the prefix before it looks for the error, so summary.json's failure site and message are unchanged.
  • Serial lane (scripts/ci-browser.sh): script output lines get a date -Is prefix.
    • PASS … / FAIL … lines, the TIMING … lines and the summary table keep their exact format.
    • The script's exit code still reaches TIMING and the lane total: the script runs set -euo pipefail, so the pipe keeps it.

Part of #897: its second item (the empty fork-PR failures of 2026-10-06) is an investigation of those runs, not a code change, and stays open.

Checks

  • Unit tests: HERDR_TEST_MODE=unit bun test scripts/ci-browser.test.ts scripts/ci-browser-parallel.test.ts: 5 pass, 0 fail.

    • Serial lane (stubbed bun): a server line is prefixed. The PASS/FAIL lines, the TIMING … exit=7 line, the summary row and Browser lane total: … exit=7 are unchanged.
    • Parallel runner: the per-script log lines are prefixed, including a partial last line, and failureSite() still finds the error.

    With the prefixes removed, the parallel test fails.

  • bun run typecheck: passes.

  • bun run check fast: passes (unit 6259 pass, 0 fail, 13 skip in 444 files, 8 at a time, 58.9s).

  • Proof on CI: this PR's own browser jobs are the end-to-end check. Their uploaded browser-logs should show the timestamps.

Outside PRs that touch the same files

The outside PR #1652 (Copilot native chat) also edits scripts/ci-browser.sh and scripts/ci-browser.test.ts and will need a rebase after this one.

No changelog fragment: CI only.

Timestamp browser script output in serial and parallel lanes while preserving lane summaries and failure-site parsing.
Add regression coverage for timestamped output and unchanged summary formats.
Part of #897
@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 0bc6691 and c9e1b05.

📒 Files selected for processing: 4
  • scripts/ci-browser-parallel.test.ts
  • scripts/ci-browser-parallel.ts
  • scripts/ci-browser.sh
  • scripts/ci-browser.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@devswha

devswha commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Merge pass. A gate reviewer on a different model than the author's reviewed this CI-tooling change: nothing blocking.

  • Tests: ci-browser.test.ts and ci-browser-parallel.test.ts pass (5 tests), and the typecheck passes.
  • Serial lane: stderr and partial lines get the prefix, and exit code 7 still reaches TIMING and the lane total.
  • Parallel runner: split UTF-8 and all 20,002 records across both streams survive, both on a normal failure and on SIGTERM.
  • Failure site: failureSite() still finds the error behind the prefix.

Notes, not blocking:

  • In the serial lane, date -Is per line costs about 11 s per 10,000 lines. CI runs the parallel runner instead. A bash printf '%(...)T' would avoid the fork if that lane gets chatty.
  • Cross-stream line order isn't guaranteed, which was already true before.

CI: the first run failed one integration test, server/submit.contract.test.ts "sends one held-copy resend across two connections" (its recorder barrier timed out after 5 s). This PR doesn't touch that code; it only changes the browser lane scripts. The re-run of the failed job passed.

CodeRabbit was rate-limited and did not review this PR, so the gate review above stands in for it. All required checks are green and no review thread is open.

@devswha
devswha merged commit 96eff68 into main Oct 23, 2026
15 of 17 checks passed
@devswha
devswha deleted the fix/browser-lane-server-timestamps branch October 23, 2026 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant