Skip to content

Add a reusable Graphile performance harness - #1813

Merged
Zetazzz merged 1 commit into
mainfrom
test/performance-harness-core-single-commit
Sep 8, 2026
Merged

Add a reusable Graphile performance harness#1813
Zetazzz merged 1 commit into
mainfrom
test/performance-harness-core-single-commit

Conversation

@Zetazzz

@Zetazzz Zetazzz commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a reusable Graphile performance harness that runs each measurement in a fresh Node process, isolating build timing and memory samples between cases. Suites supply serializable case definitions and a dedicated worker entry for their build and validation logic. This consolidates #1716 and incorporates subsequent review fixes.

  • Seeded scheduling and repetitions; build-only timing; memory snapshots, deltas, and peak RSS.
  • Schema-group/hash validation, runtime query checks, per-case validation, and a minimal upstream Graphile baseline worker.
  • Explicit database URL and worker-config CLI arguments; atomic JSON reports with database URL redaction, including custom-worker errors.
  • Configurable worker deadline (five minutes by default). A timed-out worker is killed and confirmed closed before continuing; unconfirmed cleanup after another five seconds stops scheduling and produces a failed partial report.
  • PostgreSQL fixture preparation that closes the pool on failure and preserves primary, rollback, release, and shutdown errors.
  • The baseline worker emits one terminal result after releasing its service. Measurement and release failures both fail the run, preserving both redacted diagnostics when they occur together.
  • Separate library and CLI entries: CJS/ESM library imports do not start the CLI; the built cperf executable and local workspace script run the command interface.
  • The private package is included in the CI unit-test matrix; its tests build the package before exercising generated entries.

Validation

  • Frozen-lockfile install; CJS and ESM builds.
  • 65 tests across 7 suites, covering worker timeouts, cleanup failures, library imports, and the built CLI.
  • Local workspace CLI smoke test and type checks for the new tests.
  • ESLint, Prettier check, and git diff --check.
  • Full CI on 27f4dc7331e5: 17/17 jobs passed, including 65/65 perf-harness tests.

@tenki-reviewer

tenki-reviewer Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review complete. 🟡 1 medium

💬 Inline comments (1)

🧹 Nitpicks (2) — 🟢 2 low
  • 🟢 require.main throws in the ESM entry (index.ts:13) — src/index.ts line 13 runs require.main === module, but tsconfig.esm.json emits the same file as ESM (module: esnext) into dist/esm, which package.json exposes as module: esm/index.js; in ESM scope require is undefined, so importing the module entry throws ReferenceError: require is not defined at module load.
  • 🟢 cperf bin points at nonexistent root index.js (package.json:10) — packages/perf-harness/package.json line 10 declares bin.cperf as index.js and lines 6-8 set main/module/types to package-root paths, but tsconfig.json sets outDir: "dist", so the build lands in dist/index.js and dist/esm.

This PR introduces a new performance test harness package under packages/perf-harness: a CLI (cperf), a worker process runner, seeded scheduling, metrics aggregation, report generation, and a Postgres fixture helper, together with unit tests and a CI job. The code is well-structured and well-tested, but the review surfaced a few packaging and cleanup issues.

Files Change
src/index.ts, src/stock-worker.ts, package.json, tsconfig*.json Adds the harness entry points and build config; the ESM build exposes require.main and the cperf bin points at a nonexistent root path.
src/stock-worker.ts, src/process.ts, src/run.ts Implements worker spawning and result collection; a cleanup failure can overwrite a valid success result.
src/metrics.ts, src/schedule.ts, src/report.ts, src/fixture.ts Adds metrics, scheduling, reporting, and DB fixture logic with unit tests.
__tests__/*, .github/workflows/run-tests.yaml Adds tests for the harness and wires it into CI.

Reviewed commit: 8f89a88

@tenki-reviewer tenki-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Adds a new packages/perf-harness package (CLI, worker process, metrics, scheduling, reporting, DB fixtures) plus its tests and CI wiring, with three packaging/cleanup defects to address.

Key findings

Comment thread packages/perf-harness/src/stock-worker.ts Outdated
@Zetazzz
Zetazzz force-pushed the test/performance-harness-core-single-commit branch from 8f89a88 to 27f4dc7 Compare September 7, 2026 09:21
@Zetazzz
Zetazzz merged commit 9928d93 into main Sep 8, 2026
20 checks passed
@Zetazzz
Zetazzz deleted the test/performance-harness-core-single-commit branch September 8, 2026 00:49
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