Skip to content

test: stop two Windows subprocess tests timing out at 10s - #1981

Merged
clay-good merged 2 commits into
mainfrom
claude/windows-tests-ci-green-3937dc
Sep 25, 2026
Merged

clay-good merged 2 commits into
mainfrom
claude/windows-tests-ci-green-3937dc

Conversation

@clay-good

@clay-good clay-good commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Status: LGTM. Test-only: one line in each of two test files. No product code, no changeset.

What was wrong: The tests themselves are fine. They time out because starting a new process on the Windows CI runner is slow and uneven: Windows process creation is expensive, and many test workers compete for the same machine. A test that normally takes about 3 s can take over 10 s when the machine is busy, which trips vitest's 10 s default and turns main red at random.

Test Usually takes on Windows Timed out
update-flake-script.test.ts (3 bash spawns) 0.3–4.2 s 4 times, including main after #1953 (13.1 s)
cli-e2e/basic.test.ts Git-clone init test (3 CLI + 4 git) ~2.6 s 3 times, including the #1953 merge queue

How it was fixed: Each test now gets a 60 s timeout, the same per-test override the repo already uses for other tests that start processes. The assertions are unchanged.

Proof: Both files pass locally (39/39). The Windows job on this PR is the real check.

Notes: store/git-probe-limits was already fixed by #1900. store-remove-nested failed once and already has 30 s, so I left it alone.

🤖 Generated with Claude Code

The Windows runner took 13.1s to spawn bash three times on the Version
Packages push to main, tripping the 10s default. The same test ran in
0.3s and 4.2s on the two previous main runs; nothing in the code changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@clay-good
clay-good requested a review from a team as a code owner September 25, 2026 15:42
@clay-good
clay-good requested review from TabishB and removed request for a team September 25, 2026 15:42
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: Fission-AI/OpenSpec/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 77c187ad-0f29-4ac8-bdec-997773aac5cc

📥 Commits

Reviewing files that changed from the base of the PR and between 9c1641f and d50246b.

📒 Files selected for processing (1)
  • test/cli-e2e/basic.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The fixture and Git-clone tests now use 60-second timeouts. Comments describe their observed subprocess runtimes on Windows.

Changes

Test Timeout Updates

Layer / File(s) Summary
Set test timeouts
test/update-flake-script.test.ts, test/cli-e2e/basic.test.ts
Both tests now use 60-second timeouts. Comments record the observed Windows runtime of the three Bash spawns and seven subprocesses, respectively.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to d5024

The change only gives two subprocess-heavy tests more time to complete; no specific merge-blocking risk is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to d5024

The change affects 1 system.

Changed systems: test

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — test (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in test/update-flake-script.test.ts: The fixture test now has a 60-second timeout instead of the default and documents the observed Windows runtime range for its three Bash spawns.
  • observed — Modified behavior in test/cli-e2e/basic.test.ts: The Git-clone test now has a 60-second timeout; a comment records that its seven subprocesses can take about 2.6 seconds on a Windows runner and exceed 10 seconds under load.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: increasing timeout limits for two Windows subprocess tests that were timing out at 10 seconds.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openspec-cloud

Copy link
Copy Markdown
Contributor

No PR-relevant drift confirmed.

AI-generated · A citation proves the line exists, not that it makes the case — verify before acting.
No issue was confirmed at 9c1641f; 1 requirement could not be verified.
This is not a full-repository clean result; see the check for coverage and any broader findings.
View results · Click Refresh, then Scan again in the check. Or comment /openspec-cloud.

Timed out at the 10s default on windows-pwsh three times (#1953 merge
queue, two changeset-release runs); it normally takes ~2.6s there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@clay-good clay-good changed the title test(flake): give the bash-spawning scope test a 60s timeout test: stop two Windows subprocess tests timing out at 10s Sep 25, 2026

@alfred-openspec alfred-openspec left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the two timeout-only test changes. The 60s overrides match existing subprocess-heavy test conventions, and all CI checks, including Windows, pass.

@clay-good
clay-good added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit 79b6aa9 Sep 25, 2026
17 checks passed
@clay-good
clay-good deleted the claude/windows-tests-ci-green-3937dc branch September 25, 2026 17:20
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