Skip to content

fix(ci): surface test failures in logs and de-flake perf tests - #62

Merged
mrsimpson merged 1 commit into
mainfrom
fix/flaky-tests-and-reporting
Aug 21, 2026
Merged

fix(ci): surface test failures in logs and de-flake perf tests#62
mrsimpson merged 1 commit into
mainfrom
fix/flaky-tests-and-reporting

Conversation

@ffflorian

Copy link
Copy Markdown
Collaborator

Problem

Two separate issues on main:

1. Test failures are invisible in CI logs. scripts/test-summary.js ran each package with stdio: "pipe" and printed only scraped counts, discarding all failure output. The failing run on main showed just ❌ @codemcp/knowledge-mcp-server: 21/22 passed.

2. Worse — the script could report red as green. Pass/fail was derived from scraped numbers, and the regexes didn't match vitest output containing skipped tests (Tests 1 failed | 22 passed | 2 skipped (25)). Unmatched packages were silently dropped from the report and the totals. Verified against the old script with a deliberately failing test: it printed 🎉 All tests completed successfully! and exited 0.

3. Flaky perf assertions in packages/mcp-server/src/__tests__/performance.test.ts:

  • expect(time).toBeLessThan(10) on a single cold sample, including module init and JIT warm-up — 2.29ms locally, under 4x headroom on a shared CI runner.
  • expect(memoryDiff).toBeLessThan(10MB) on a single heapUsed delta — reported -0.36MB locally, i.e. GC noise dominated the signal entirely.

Changes

  • Test runner streams child output to stdout/stderr while buffering it, and derives success from exit codes, never parsing. Parsing is presentation-only, so an unrecognised summary line can no longer flip a red run green. Failed suites are named explicitly at the end.
  • Perf timing: warm up, then assert median of 9 samples against 50ms.
  • Perf memory: assert aggregate growth across 50 creations with a 1MB-per-server budget.
  • Root test now runs turbo run build first. packages/cli integration tests exec dist/index.js, but the root script bypassed turbo so its existing dependsOn: ["build"] never applied — a stale dist produced failures unrelated to the current source.
  • Added dist/ to .prettierignore (root + per package). Prettier was checking build artifacts; CI only passes today because format:check happens to run before build.

Verification

  • Failing-test injection now yields exit 1, the suite named, and the full assertion diff, file, line and code frame in the log.
  • Skipped-test counts parse correctly (22/25 passed, 1 failed, 2 skipped).
  • Perf tests reproducible across runs: median 0.13–0.17ms, 92.6–92.7KB per server (vs. the old ±0.36MB swing).
  • Full CI sequence green locally: format:check:all, lint:all, build, test — 265 tests, 4 consecutive runs.

Not verified: behaviour on a genuinely contended CI runner. The headroom argument is inference from local numbers, not a measurement.

@ffflorian
ffflorian requested a review from mrsimpson August 21, 2026 08:52
@mrsimpson

Copy link
Copy Markdown
Collaborator

Ouch... that was really poor earlier.

@mrsimpson
mrsimpson merged commit 3c8ea92 into main Aug 21, 2026
1 check passed
@mrsimpson
mrsimpson deleted the fix/flaky-tests-and-reporting branch August 21, 2026 09:25
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