Skip to content

ci: reduce test workflow setup time#11409

Open
markijbema wants to merge 5 commits into
mainfrom
ci/test-cache-ab
Open

ci: reduce test workflow setup time#11409
markijbema wants to merge 5 commits into
mainfrom
ci/test-cache-ab

Conversation

@markijbema

@markijbema markijbema commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Reduce test workflow wall time by making Turbo caching functional, skipping a counterproductive Windows Bun package-cache restore/save, and requiring immutable dependency installs.

The previous Turbo cache path did not exist after test execution, so no task results were persisted. Caching Turbo's current default .turbo/cache path allows unchanged unit-test and prerequisite-build tasks to be reused across commits on the same OS. Tasks whose source, tests, dependencies, configuration, or declared hashed environment change still execute normally.

Benchmark results

  • Windows Setup Bun without the Bun package cache: 1m27s
  • Windows Setup Bun with the roughly 1 GB Bun package cache: 2m23s
  • Final no-cache setup validation: 1m31s
  • Unchanged Windows Turbo graph: all 10 tasks restored in 163ms, replaying a prior successful run of 486 files / 5,233 tests

The A/B result favors skipping both restore and save of the Bun package cache on Windows while retaining it on Linux. --frozen-lockfile prevents CI installs from silently changing the lockfile.

This matches upstream OpenCode's intended Turbo policy: unit tests are cacheable and the workflow restores prior OS-specific task results across commits. Upstream currently fails to persist those results because it still points the cache action at the obsolete node_modules/.cache/turbo directory.

Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Comment thread .github/workflows/test.yml Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review (7fd4ac2)

The new commit adds cache: false, // kilocode_change to every test:ci task in turbo.json (generic test:ci, @kilocode/cli#test:ci, @kilocode/kilo-gateway#test:ci, @kilocode/kilo-jetbrains#test:ci, @opencode-ai/ui#test:ci). This guarantees tests execute on every CI run rather than being replayed from cached JUnit artifacts — matching the PR intent and the prior benchmark showing Turbo replayed all ten tasks in 163ms without running tests.

The previously carried-forward WARNING about the .turbo/cache cache-action path is now resolved/invalid:

  • With cache: false on all test:ci tasks, test results are never cached, so the cache path only affects prerequisite build tasks.
  • .turbo/cache is in fact the correct default cache directory for Turbo v2 (this repo pins turbo@2.9.14 and uses the v2-8-13 schema). The v1 default node_modules/.cache/turbo was moved to .turbo/cache in v2, so TURBO_CACHE_DIR does not need to be set. The original warning was a false positive.

Minor note (not blocking): with cache: false, the outputs: [".artifacts/unit/junit.xml"] entries on the test:ci tasks become no-ops, since Turbo ignores outputs when caching is disabled. Harmless and reasonable to keep for documentation/future re-enabling.

Resolved since last review
File Line Issue Resolution
.github/workflows/test.yml 72 Turbo cache path set to .turbo/cache but TURBO_CACHE_DIR never set ✅ Invalid — .turbo/cache is the Turbo v2 default cache dir; no env var needed. Additionally the new commit disables caching for all test:ci tasks, so test results never rely on the cache.
Files Reviewed (1 file in incremental diff)
  • turbo.json - adds cache: false to all test:ci tasks; no new issues

Fix these issues in Kilo Cloud

Previous Review Summaries (5 snapshots, latest commit 3e51e3c)

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

Previous review (commit 3e51e3c)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
.github/workflows/test.yml 72 Turbo cache path is set to .turbo/cache but TURBO_CACHE_DIR is never set, so Turbo likely still writes to node_modules/.cache/turbo and the cache action saves/restores an empty directory — defeating the goal of persisting task results. (carried forward, unchanged in latest commit)
Latest commit (3e51e3c)

The new commit is comment-only on .github/actions/setup-bun/action.yml: it adds explanatory comments documenting the Windows cache-restore/save timing rationale (2m23s restore vs 1m27s fresh install) and adds # kilocode_change markers to the modified lines. No behavioral change — the carried-forward Turbo cache issue in test.yml is the only outstanding item.

Resolved since last review
File Line Issue Resolution
.github/actions/setup-bun/action.yml 63 Save/Restore guard asymmetry — Save step lacked the runner.os != 'Windows' guard present on Restore ✅ Fixed in b96ef37599 — Save step now includes runner.os != 'Windows' &&, matching the Restore guard. Windows no longer uploads an unused ~1GB cache.
Files Reviewed (1 file in incremental diff)
  • .github/actions/setup-bun/action.yml - comment-only additions; no new issues
  • .github/workflows/test.yml - 1 issue carried forward (Turbo cache path; existing inline comment, not duplicated)

Fix these issues in Kilo Cloud

Previous review (commit b96ef37)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
.github/workflows/test.yml 72 Turbo cache path is set to .turbo/cache but TURBO_CACHE_DIR is never set, so Turbo likely still writes to node_modules/.cache/turbo and the cache action saves/restores an empty directory — defeating the goal of persisting task results. (carried forward, unchanged in latest commit)
Resolved since last review
File Line Issue Resolution
.github/actions/setup-bun/action.yml 63 Save/Restore guard asymmetry — Save step lacked the runner.os != 'Windows' guard present on Restore ✅ Fixed in b96ef37599 — Save step now includes runner.os != 'Windows' &&, matching the Restore guard. Windows no longer uploads an unused ~1GB cache.
Other Observations (not in diff)
  • The new commit re-adds # kilocode_change markers to the modified lines in both .github/actions/setup-bun/action.yml and .github/workflows/test.yml. These are additive comments and don't affect behavior.
  • --frozen-lockfile on bun install (from an earlier benchmark commit) is retained — a good correctness improvement that hard-fails CI on out-of-sync lockfiles.
Files Reviewed (2 files in incremental diff)
  • .github/actions/setup-bun/action.yml - previous suggestion resolved; new marker comments only
  • .github/workflows/test.yml - 1 issue carried forward (Turbo cache path; existing inline comment, not duplicated)

Fix these issues in Kilo Cloud

Previous review (commit bbc5b7e)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
.github/workflows/test.yml 72 Turbo cache path changed to .turbo/cache but TURBO_CACHE_DIR isn't set, so Turbo likely still writes to node_modules/.cache/turbo and the cache action saves/restores an empty directory — defeating the PR's goal of persisting task results. (carried forward, unchanged in latest commit)

SUGGESTION

File Line Issue
.github/actions/setup-bun/action.yml 37 Re-adding the Windows guard on Restore leaves the matching Save step (line 63) unguarded, so Windows push-to-main uploads a ~1GB cache that this guard guarantees is never restored.
Other Observations (not in diff)
  • The new commit bbc5b7ef ("skip slower Bun cache restore on Windows") reverts the benchmark's no-guard state by re-adding if: runner.os != 'Windows' to the Restore step — i.e. the benchmark concluded Bun cache restore on Windows is slower than a fresh install, so the guard is intentionally retained. This matches the PR description's intent to "retain whichever setup is faster."
  • The --frozen-lockfile addition to bun install (from the earlier benchmark commit) is retained — a good correctness improvement that hard-fails CI on out-of-sync lockfiles.
Files Reviewed (2 files in incremental diff)
  • .github/actions/setup-bun/action.yml - 1 new issue (Save/Restore guard asymmetry)
  • .github/workflows/test.yml - 1 issue carried forward (Turbo cache path; existing inline comment, not duplicated)

Fix these issues in Kilo Cloud

Previous review (commit dd42681)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
.github/workflows/test.yml 72 Turbo cache path changed to .turbo/cache but TURBO_CACHE_DIR isn't set, so Turbo likely still writes to node_modules/.cache/turbo and the cache action saves/restores an empty directory — defeating the PR's goal of persisting task results.
Other Observations (not in diff)
  • Resolved in this commit: The Windows Bun-cache gap flagged previously is now fixed — removing if: runner.os != 'Windows' from the "Restore Bun dependencies" step means the Windows cache saved on push-to-main is now actually restored on subsequent Windows runs. Benchmark comparison is now valid.
  • Adding --frozen-lockfile to bun install is a good correctness improvement; it will hard-fail CI on out-of-sync lockfiles instead of silently updating them, which is desirable.
Files Reviewed (1 file in incremental diff)
  • .github/actions/setup-bun/action.yml - 0 new issues (Windows restore gap resolved)
  • .github/workflows/test.yml - 1 issue carried forward (unchanged in this commit)

Fix these issues in Kilo Cloud

Previous review (commit d6e63d2)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

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

WARNING

File Line Issue
.github/workflows/test.yml 72 Turbo cache path changed to .turbo/cache but TURBO_CACHE_DIR isn't set, so Turbo likely still writes to node_modules/.cache/turbo and the cache action saves/restores an empty directory — defeating the PR's goal of persisting task results.
Other Observations (not in diff)
  • .github/actions/setup-bun/action.yml: The "Save Bun dependencies" step still runs on Windows (its if checks steps.bun-cache.outputs.cache-hit != 'true', which is empty when the restore step is skipped). On push-to-main this populates a Windows Bun cache that is never restored. This matches the benchmark intent described in the PR, but if this branch merges without the follow-up "restore Windows Bun cache" commit, Windows runs will perpetually save a never-read cache — worth confirming the follow-up lands before merge.
  • Adding --frozen-lockfile to bun install is a good correctness improvement; it will hard-fail CI on out-of-sync lockfiles instead of silently updating them, which is desirable.
Files Reviewed (2 files)
  • .github/actions/setup-bun/action.yml - 0 inline issues
  • .github/workflows/test.yml - 1 issue

Fix these issues in Kilo Cloud


Reviewed by glm-5.2-20260616 · 206,092 tokens

Review guidance: REVIEW.md from base branch main

markijbema and others added 2 commits June 18, 2026 09:58
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Comment thread .github/actions/setup-bun/action.yml Outdated
markijbema and others added 2 commits June 18, 2026 10:19
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.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.

1 participant