Skip to content

test: reduce suite latency with safe sharding - #832

Open
benvinegar wants to merge 5 commits into
mainfrom
perf/test-suite-sharding
Open

test: reduce suite latency with safe sharding#832
benvinegar wants to merge 5 commits into
mainfrom
perf/test-suite-sharding

Conversation

@benvinegar

@benvinegar benvinegar commented Aug 22, 2026

Copy link
Copy Markdown
Member

Summary

  • run the default Bun test suite as two independent file shards
  • avoid Bun 1.3.14's --parallel isolation path, which breaks OpenTUI native FFI initialization
  • supervise shard failures, interrupts, partial launches, and orphan cleanup
  • keep Windows and macOS serial because the complete multi-process suite is validated and benchmarked on Linux
  • use the same package test command across PR validation jobs

Benchmark

Bun 1.3.14 on Linux with the full 3,027-test suite:

Mode Mean wall time
Serial 43.7s
Two shards 25.2s

This reduces local wall time by about 42%. Under a simulated two-vCPU affinity, two shards reduced wall time from 46.4s to 29.3s. On Linux, higher shard counts remain available through HUNK_TEST_SHARDS, while the automatic default stays capped at two.

Validation

  • bun run test
  • bun test scripts/run-test-suite.test.ts
  • bun run typecheck
  • bun run lint
  • changed-file formatting check
  • manual SIGTERM cleanup check confirming no shard descendants remain
  • Windows CI reproduction confirming --no-orphans must be omitted and daemon assertions must follow the serving child PID
  • retrying Windows temp-directory cleanup after subprocess handles are released
  • ten repeated scrollbar test runs after widening Windows timer margins

This PR description was generated by Pi using gpt-5.6-sol

@vercel

vercel Bot commented Aug 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 23, 2026 12:42am

Request Review

@greptile-apps

greptile-apps Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces the serial default Bun test command with a supervisor that runs the suite in independent file shards while avoiding Bun’s isolated parallel-worker mode.

  • Chooses up to two shards automatically, with a validated HUNK_TEST_SHARDS override.
  • Supervises launch failures, test failures, interrupts, and forced termination.
  • Routes Windows PR validation through the same package test command.
  • Adds unit coverage for shard-count validation, command construction, and termination forwarding.

Confidence Score: 4/5

The PR appears safe to merge, with one non-blocking repository-convention issue around direct environment-variable access.

The shard orchestration has no established blocking failure, but the new supervisor bypasses the prescribed validated environment interface when reading the shard override and forwarding child environments.

Files Needing Attention: scripts/run-test-suite.ts

Important Files Changed

Filename Overview
scripts/run-test-suite.ts Introduces the shard supervisor and cleanup behavior; direct process.env access conflicts with the mounted environment-access rule.
scripts/run-test-suite.test.ts Adds focused unit tests for shard-count resolution, generated commands, and tolerant signal forwarding.
package.json Routes the default test script through the new Bun shard supervisor.
.github/workflows/pr-ci.yml Updates Windows PR validation to exercise the same package-level test entry point.

Sequence Diagram

sequenceDiagram
    participant U as Caller / CI
    participant R as run-test-suite.ts
    participant S1 as Bun shard 1
    participant S2 as Bun shard 2
    U->>R: bun run test [args]
    R->>R: Resolve shard count
    par Launch file shards
        R->>S1: "bun test --shard=1/2 patterns"
        R->>S2: "bun test --shard=2/2 patterns"
    end
    alt Interrupt or partial launch
        U-->>R: SIGINT / SIGTERM
        R-->>S1: Forward signal, then SIGKILL
        R-->>S2: Forward signal, then SIGKILL
    else Normal completion
        S1-->>R: Exit code
        S2-->>R: Exit code
        R-->>U: Combined success/failure status
    end
Loading
Prompt To Fix All With AI
### Issue 1
scripts/run-test-suite.ts:76
**Direct environment access bypasses validation**

The new supervisor reads `HUNK_TEST_SHARDS` directly from `process.env` and also spreads `process.env` into each child at line 87, bypassing the repository-prescribed type-safe environment interface and establishing an inconsistent configuration-access pattern.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "test: reduce suite latency with safe sha..." | Re-trigger Greptile

Comment thread scripts/run-test-suite.ts

/** Run the default suite in independent Bun processes without enabling Bun's isolate mode. */
export async function main(args = Bun.argv.slice(2)) {
const shardCount = resolveTestShardCount(availableParallelism(), process.env.HUNK_TEST_SHARDS);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Direct environment access bypasses validation

The new supervisor reads HUNK_TEST_SHARDS directly from process.env and also spreads process.env into each child at line 87, bypassing the repository-prescribed type-safe environment interface and establishing an inconsistent configuration-access pattern.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/run-test-suite.ts
Line: 76

Comment:
**Direct environment access bypasses validation**

The new supervisor reads `HUNK_TEST_SHARDS` directly from `process.env` and also spreads `process.env` into each child at line 87, bypassing the repository-prescribed type-safe environment interface and establishing an inconsistent configuration-access pattern.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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