Skip to content

Expose CI test counts and execution costs in job summaries - #66

Merged
wesbillman merged 2 commits into
mainfrom
test-cost-reporting
Sep 16, 2026
Merged

wesbillman merged 2 commits into
mainfrom
test-cost-reporting

Conversation

@comp615

@comp615 comp615 commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

September 15 — review follow-up

🤖 Addressed the requested Playwright identity fix in this commit. Slow-test JSON now retains projectName; summary labels include it, so the same test in Chromium and WebKit remains distinguishable. Unnamed projects and Vitest retain their existing labels. Counts, durations, file aggregation, workflow selection and runner exit handling are unchanged.

Both the asymmetric synthetic regression and the real Playwright JSON-reporter probe failed before the fix for missing project names. All nine reporter/topology Node checks pass afterward, including summary output and unnamed-project handling. The real probe uses two named projects without launching browsers. Biome, diff checks, commit/push hooks and hosted DCO passed. No additional browser journeys or timing claims.

Current CI is running; reviewer re-review remains required. The previous rebased-head run passed, but that does not verify this follow-up. This PR targets main independently after #63 merged. The evidence below is historical and tied to its named snapshots.


Behavior and scope

"Independent PR against main, now including merged #63 in its base."

Add job summaries and machine-readable artifacts for Vitest and Playwright counts, failures/skips, elapsed command wall time, summed test execution time, and slowest tests/files. Distinguish per-file elapsed time from summed per-file execution. Preserve existing browser engines, shards, measurement isolation, native setup, required checks, and runner failure exit codes.

The wrapper consumes built-in JSON reporters, writes evidence after runner failure, and rejects missing/invalid/incomplete reports. No additional browser runs, dependencies, retries, timing thresholds, permissions, PR commenting bot, or baseline service. Agent guidance requires inspecting this evidence; no PR-template changes.

Cost and timing evidence

This is observability, not an optimization: no speedup is claimed. Local tiny-Vitest probes were direct 1.77/1.48/1.20s versus wrapped 1.61/1.28/1.25s. Startup variance exceeded any reliable overhead estimate; the final pair was approximately +50ms. These are not hosted CI timings. Five new Node checks include real lightweight runner-format probes (roughly 3s locally), without launching a browser.

CI job summaries and retained artifacts provide the first hosted evidence for this branch. Compare equivalent engine/shard selections and snapshots; counts and summed durations are not substitutes for critical-path elapsed time.

Verification and remaining gates

  • Full Node integration suite: 43 passed before final reporter hardening; all nine affected reporter/topology checks passed again on the final changes.
  • Full Vitest suite executed through the wrapper: 1,374 tests passed. The generated summary correctly reports 20.06s command wall time versus 88.85s summed test execution, with machine-readable artifacts. This branch targets main and does not include the additional RTL tests from the other PRs.
  • Real Vitest and Playwright JSON output is exercised, including passed/skipped/todo cases; failure exit codes, unavailable reports, CI summary-file handling, and asymmetric count/duration aggregation are checked.
  • A fail-then-pass check establishes that an incomplete report cannot turn a zero child exit into a green wrapper result.
  • Browser topology still discovers 92/90 functional cases per engine with exact coverage and no duplicates.
  • Biome and diff checks passed. Hosted summary/artifact rendering, full browser execution, and native/platform CI remain pending.

🤖 AI-assisted implementation and PR description.

Hosted CI evidence — September 14

🤖 CI verification: this completed run at PR head, compared with main baseline at baseline head. All required CI lanes, DCO and security checks passed. No PR has merged yet; post-merge main verification remains pending.

Coverage: 364 unique functional browser cases passed (182 per engine), five CI-selected measurement cases passed, and 1,374 Vitest tests passed. Runner totals contain no skipped or flaky cases. Test identities were compared across logs, including colocated workflow journeys; #64's two intentional title changes were normalized. Three documented local-only WebKit measurement cases remain excluded from CI, unchanged. Measurement artifacts retain the existing narrow WebKit ResizeObserver warning exception; no broader allowance was added.

Measured interval Baseline This run
chromium 1/2 — Functional journeys 367s 309s
chromium 2/2 — Functional journeys 343s 406s
webkit 1/2 — Functional journeys 496s 344s
webkit 2/2 — Functional journeys 491s 504s
All Vitest tests 61s 51s
Serial browser measurements 127s 131s
Earliest job start → completed CI required gate 619s 582s

Times are Actions step durations, not summed test durations. The overall interval includes setup/build and scheduling between jobs but excludes initial queue time. Linux lanes use the pinned Ubuntu 24.04 workflow; runner/cache variation and workload distribution still affect comparisons.

The unchanged 92/90 allocation and exact baseline functional test identities remain. Downloaded all six timing artifacts: four browser shards, measurements and Vitest. Counts, zero failures/skips and complete status match runner logs. Wrapper elapsed times track Actions test-step durations within about one second; summed test time is separately labeled. Browser step timings are mixed versus baseline, so this visibility-only PR does not establish a speedup or a reliable isolated overhead estimate.

@comp615
comp615 marked this pull request as ready for review September 15, 2026 14:27
@comp615
comp615 requested review from a team and wesbillman as code owners September 15, 2026 14:27
Co-authored-by: Amp <amp@ampcode.com>
Signed-off-by: Charlie Croom <ccroom@squareup.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0a149-b278-7550-acc4-356168b0c4b5
@comp615
comp615 force-pushed the test-cost-reporting branch from 4f43b66 to bb3dc9d Compare September 15, 2026 15:12

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

Reviewed head bb3dc9d7af3e4d7b56201df70968f37cd76a61ce against base d21f43364fe9b7bdf2643779dab819fd9e8c16bb.

[P2] Retain the Playwright project in slow-test identities

scripts/ci-test-report.mjs:87-95 discards entry.projectName, and the summary renders only the remaining title/file. The measurement job intentionally runs both engines in one report, so this loses information needed to attribute a slow result.

This is visible in the existing exact-head CI run, browser-measurements artifact ID 10404485967: raw ci-report.json identifies scroll.spec.mjs:96 as chromium-measurements: 12,188 ms and webkit-measurements: 21,092 ms. The generated ci-timing.json top-five contains both durations under the identical “real scrolling preserves reading intent over append, community/channel switches and reload” title/file, with no engine. The job-summary formatter therefore also produces indistinguishable identities. Readers cannot tell which engine needs attention without abandoning the new summary and rejoining the raw report.

Smallest fix / exit criterion: retain and render the project name with each slow-test identity; add a regression showing that the same case from two projects remains distinguishable in the machine-readable evidence and summary. No new reporting machinery or broader title-path redesign is needed.

Evidence and scope

  • The unchanged engine/shard selections, measurement isolation, native setup, strict required gate and ordinary runner-failure handling were reviewed end to end. Production source, test cases, dependency pins and browser configs are unchanged by this diff.
  • Independently reconciled all six exact-head raw/derived artifacts: 1,549 Vitest tests, 382 functional browser cases across both engines, and five measurement cases; their counts and summed execution times match. The successful run is separate from an earlier cancelled duplicate. Accurate totals do not repair the missing identity above.
  • Source/metadata/artifact-only review with independent reporter and workflow lanes. No local checkout, dependency installation, import, build or test execution; no native/UI acceptance or speedup claim.

Co-authored-by: Amp <amp@ampcode.com>
Signed-off-by: Charlie Croom <ccroom@squareup.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a0a149-b278-7550-acc4-356168b0c4b5
@comp615
comp615 requested a review from wesbillman September 15, 2026 16:24

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Follow-up: previous P2 resolved

No remaining blockers found in the focused re-review of 2dd1bebd680717587ba4e6fef780fab784100876 against d21f43364fe9b7bdf2643779dab819fd9e8c16bb.

  • Project identity now survives end to end. parsePlaywright retains projectName, timing JSON serializes it, and the summary prefixes the test title with the project. The asymmetric regression checks both serialized identities and summary labels; the built-in reporter probe covers two named projects. Unnamed-project and Vitest labels retain their prior behavior. This satisfies the exit criterion in my previous review.
  • Verified current hosted evidence. All six artifact ZIPs from CI run 34992224408 match GitHub’s recorded SHA-256 digests. Raw and derived reports reconcile 1,549 Vitest tests, 382 functional browser cases, and five measurement cases, including summed durations and top-five test/file rows. The previously ambiguous scrolling measurement now retains webkit-measurements at 22,589 ms and chromium-measurements at 11,939 ms. An exact-source formatter probe produces distinct engine labels and separate 131.40 s wall / 122.80 s summed execution for measurements. Current hosted checks are successful, including the required gate, integration, all browser lanes, security and DCO.
  • Scope and limits. The two-file follow-up leaves execution selection, counts, duration aggregation, workflow commands and failure handling unchanged. Independent source review also returned clear. No local suite, browser, native GUI or full just scan was rerun; generated Markdown was checked, not GitHub’s visual rendering. Hosted CI still does not establish the three documented local-only WebKit cases. This is a follow-up comment, not an approval or dismissal of the earlier review.

@wesbillman
wesbillman merged commit c3e36a1 into main Sep 16, 2026
12 checks passed
@wesbillman
wesbillman deleted the test-cost-reporting branch September 16, 2026 02:33
delkc added a commit that referenced this pull request Sep 16, 2026
* origin/main:
  Reduce navigation fixture cost and stabilize typing thread opening (#68)
  Key browser downloads by the installed Playwright version (#75)
  Expose CI test counts and execution costs in job summaries (#66)
  Right-size notification fixtures and stabilize thread-navigation coverage (#69)
  Apply shared design foundations throughout the Buzz app (#59)
  Generate only the history needed by navigation and plugin journeys (#65)
  Move stale completion permutations into mounted React tests (#64)

Signed-off-by: Clay Delk <clay.delk@gmail.com>
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.

2 participants