Skip to content

test(ci): shard cross-platform CLI suite - #12051

Merged
marius-kilocode merged 10 commits into
mainfrom
hammerhead-door
Jul 10, 2026
Merged

test(ci): shard cross-platform CLI suite#12051
marius-kilocode merged 10 commits into
mainfrom
hammerhead-door

Conversation

@marius-kilocode

@marius-kilocode marius-kilocode commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

The CLI unit suite starts each test file in a separate Bun process to isolate PID-scoped databases, environment state, child processes, and teardown. The full-platform workflow was dominated by process startup and long process-heavy tails, while the Linux HttpApi gate ran only after the unit queue.

This preserves process-per-file isolation, retries, JUnit reporting, and complete eligible coverage on Linux and Windows. Files are ordered largest-first and assigned exactly once through deterministic greedy balancing across two Linux shards and four Windows shards. macOS runs one curated 106-file Darwin-native profile covering every sandbox test plus process, terminal, PTY, watcher, filesystem, permission, Git, worktree, and native-runtime behavior. Platform-neutral coverage remains backed by complete Linux and Windows runs. Non-CLI package tests still run once per OS, and the full HttpApi exerciser runs independently so it overlaps unit jobs.

Windows fixture cleanup forces GC only after a locked-directory failure instead of before every deletion. Persistent-process tests wait for child exit and persisted output before deleting fixtures or simulating reload. Turbo cache lookup excludes installed package manifests, includes shard identity, and shared Linux sandbox setup lives in one composite action. Stable aggregate checks preserve the existing unit (linux) and test (linux) required contexts while shard jobs provide detailed failures.

Measured impact

Cold reference: https://github.com/Kilo-Org/kilocode/actions/runs/28947466351
Cold sharded run: https://github.com/Kilo-Org/kilocode/actions/runs/29031811351
Final profile-trim run: https://github.com/Kilo-Org/kilocode/actions/runs/29082265682

Path Before After Saved Faster
Complete test workflow 18m44s 11m10s 7m34s 40.4%
Linux critical path 12m24s 6m40s 5m44s 46.2%
Windows critical path 18m02s 7m31s 10m31s 58.3%
macOS curated profile 7m26s / 137 files 5m52s / 106 files 1m34s 21.1%

The macOS profile comparison is measured on consecutive revisions of this branch. The original full-suite macOS reference was 13m20s, but that comparison is not coverage-equivalent because platform-neutral tests are now intentionally covered by the complete Linux and Windows runs.

Comment thread .github/workflows/test.yml Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Since the last review, the branch picked up the test(cli): normalize profile fixture paths commit, which changes a single line in packages/opencode/test/kilocode/test-profile.test.ts: the raw glob-scanned file list (all) now has its path separators normalized before being sorted and used in assertions. Bun.Glob.scan returns OS-native separators, so on Windows the previous all array still contained backslashes, which could break the startsWith("kilocode/sandbox/") filter and the subsequent array-equality assertion on lines 20-21, even though TestProfile.resolve itself already normalizes backslashes internally (covered by the existing "normalizes Windows test paths" test). The fix is minimal, test-only, and contained to a kilocode/-named path, so no kilocode_change markers are required.

The two pre-existing open findings on .github/workflows/test.yml (sandbox-helper setup duplication, and macOS test-profile scope) are unrelated to this commit's file and remain tracked in their existing inline comments.

Files Reviewed (1 file changed since last review)
  • packages/opencode/test/kilocode/test-profile.test.ts
Previous Review Summaries (6 snapshots, latest commit 6c96179)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 6c96179)

Status: No Issues Found | Recommendation: Merge

Since the last review, the branch picked up the test(ci): trim platform-neutral macOS coverage commit. It further narrows the Darwin CI profile (packages/opencode/script/kilocode/test-profile.ts) by removing additional platform-neutral test globs (e.g. cli/run/{footer.view,runtime.stdin,scrollback.surface}, several cli/tui/* snapshot tests, tool/external-directory, util/wildcard, several single-file kilocode/* entries, mcp/lifecycle, shell/*, and tightening server/httpapi-pty* to the exact httpapi-pty.test.ts file) that remain fully covered by the complete Linux/Windows unit runs. The companion test (test/kilocode/test-profile.test.ts) adds matching assertions confirming these files are now excluded from the darwin profile while the kilocode/sandbox/* group still resolves to every sandbox test file. The change is self-contained to these two kilocode-named files (no kilocode_change markers required) and consistent with the profile-trimming pattern already in place.

The pre-existing open finding on macOS test-profile scope (.github/workflows/test.yml line 143) is unrelated to this commit's files and remains tracked in its existing inline comment.

Files Reviewed (2 files changed since last review)
  • packages/opencode/script/kilocode/test-profile.ts
  • packages/opencode/test/kilocode/test-profile.test.ts

Previous review (commit b0623a6)

Status: No Issues Found | Recommendation: Merge

Since the last review, the branch picked up the fix(ci): restore required unit status commit. It adds a new unit-required job whose GitHub check name is pinned to the literal string unit (linux), decoupled from the sharded/dynamic unit matrix job names (which now render as unit (unchanged) when general tests are skipped, instead of colliding with that literal name). This restores a stable, always-present required-status-check name for branch protection while the actual unit matrix job names remain dynamic across shards/OSes. The change is small, self-contained to the kilocode_change CI block, and consistent with the existing required (test (linux)) job's if: always() + needs.<job>.result == 'success' gating pattern.

The pre-existing open finding on macOS test-profile scope (line 143) is unrelated to this commit and remains tracked in its existing inline comment.

Files Reviewed (1 file changed since last review)
  • .github/workflows/test.yml

Previous review (commit c340790)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
.github/workflows/test.yml 143 macOS CI now runs only the curated darwin test-profile subset (single shard) instead of the complete suite; PR description still describes a third complete macOS shard and full-suite timings

Since the last review, the branch picked up the test(ci): focus macOS on native coverage commit, which drops macOS from three full-suite shards down to one shard scoped to TestProfile's curated Darwin-native file list via the new KILO_TEST_PROFILE=darwin env var, and hardened two flaky persistent-process tests (background-process.test.ts now waits for log flush before reload and for a descendant readiness file before reading its pid, watcher.test.ts now also runs under the macOS profile). These test changes look correct and well-justified, but the PR description's narrative and measured-impact numbers no longer match the shipped macOS scope, which is worth reconciling before merge.

Files Reviewed (5 files changed since last review)
  • .github/workflows/test.yml
  • packages/opencode/script/kilocode/test-profile.ts
  • packages/opencode/test/file/watcher.test.ts
  • packages/opencode/test/kilocode/background-process.test.ts
  • packages/opencode/test/kilocode/test-profile.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 736850e)

Status: No Issues Found | Recommendation: Merge

The previously flagged duplication of the Zig/bwrap sandbox-helper setup between the unit and httpapi jobs has been resolved by extracting the shared steps into the new composite action .github/actions/setup-linux-sandbox, matching the existing ./.github/actions/setup-bun pattern. The matrix identity fields were renamed from a single name/shard string to structured os/index/total fields (propagated consistently through the job name, Turbo cache key, artifact name, and KILO_TEST_SHARD), a third macOS shard (3/3) was added as described in the PR, and the CLI test-ci Turbo task now also keys its cache on the workflow file itself.

Files Reviewed (3 files)
  • .github/actions/setup-linux-sandbox/action.yml
  • .github/workflows/test.yml
  • turbo.json

Previous review (commit 6339835)

Status: No Issues Found | Recommendation: Merge

The previously flagged duplication of the Zig/bwrap sandbox-helper setup between the unit and httpapi jobs has been resolved by extracting the shared steps into a new composite action, .github/actions/setup-linux-sandbox. Both jobs now reference it via uses: ./.github/actions/setup-linux-sandbox, matching the existing ./.github/actions/setup-bun pattern. This change also adds a third macOS shard (macos-3, 3/3) to further balance the macOS critical path, consistent with the PR description.

Files Reviewed (2 files)
  • .github/actions/setup-linux-sandbox/action.yml
  • .github/workflows/test.yml

Previous review (commit 443d510)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
.github/workflows/test.yml 195 Zig/bwrap sandbox-helper setup steps are duplicated verbatim between the unit and new httpapi jobs; consider a composite action to keep them in sync
Files Reviewed (7 files)
  • .github/workflows/test.yml
  • packages/opencode/script/kilocode/test-shard.ts
  • packages/opencode/script/test-runner.ts
  • packages/opencode/test/kilocode/background-process.test.ts
  • packages/opencode/test/kilocode/cleanup.ts
  • packages/opencode/test/kilocode/test-shard.test.ts
  • turbo.json

The sharding matrix, TestShard.split/order load-balancing logic, the httpapi job split-out with its required-job gating, the Windows GC-on-first-lock-failure change, and the child-process exit synchronization fix in background-process.test.ts all check out correctly against the CI/turbo config and the PR's stated intent.

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5-20260630 · Input: 38 · Output: 9.9K · Cached: 904K

Review guidance: REVIEW.md from base branch main

Comment thread .github/workflows/test.yml
@marius-kilocode

Copy link
Copy Markdown
Collaborator Author

Final observed pipeline timing

Compared the original reference workflow with the final green profile-trim workflow using the GitHub workflow createdAt to updatedAt timestamps:

Path Before Final observed Saved Faster
Complete test workflow 18m44s 6m50s 11m54s 63.5%
Linux critical shard 12m24s 4m22s 8m02s 64.8%
Windows critical shard 18m02s 5m59s 12m03s 66.8%
macOS curated profile 7m26s / 137 files 5m52s / 106 files 1m34s 21.1%

Reference: https://github.com/Kilo-Org/kilocode/actions/runs/28947466351

Final run: https://github.com/Kilo-Org/kilocode/actions/runs/29082265682

Linux and Windows still run the complete eligible CLI suite. macOS runs the curated native-sensitive profile and retains all 11 sandbox test files. The 6m50s figure is the latest observed end-to-end workflow time, not labeled as a controlled cold-cache measurement because cache state may differ between workflow runs.

@marius-kilocode
marius-kilocode merged commit d6d9076 into main Jul 10, 2026
28 checks passed
@marius-kilocode
marius-kilocode deleted the hammerhead-door branch July 10, 2026 09:56
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
test(ci): shard cross-platform CLI suite
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.

3 participants