Repository navigation
fix(test): isolate concurrent interop harness runs - #4228
Conversation
Co-Authored-By: GPT-6 <noreply@openai.com>
Co-Authored-By: GPT-6 <noreply@openai.com>
Run one full matrix in CI instead of two concurrent ones in the same checkout (they clobber each other's Python venv and maturin output), keep the Playwright check out of the workspace bun test, and complete the quest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 17 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (13)
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Landing summary:
Confirmed this fixes the parallel-worktree case: reservations now share (Written by Claude Opus 5.5) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3d439a833
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| echo "building go client (workspace moq-go via uniffi-bindgen-go)..." | ||
| local staged ffi_pkg wrapper_pkg src="$HARNESS_RUN/go-client" | ||
| if ! staged=$(bash "$WORKSPACE/go/scripts/stage.sh" 2>"$HARNESS_RUN/go-stage.log"); then | ||
| if ! staged=$(bash "$WORKSPACE/go/scripts/stage.sh" --output "$HARNESS_RUN/go-stage" 2>"$HARNESS_RUN/go-stage.log"); then |
There was a problem hiding this comment.
Keep Python staging isolated before closing the quest
When two full matrices run from the same checkout, this change isolates only Go: prepare_python still runs just py build, whose uv sync, maturin develop, and editable install concurrently rewrite the same $WORKSPACE/.venv and generated Python bindings. Those runs can therefore still clobber each other even though this commit deletes the concurrency quest; move the Python environment and generated output under HARNESS_RUN, or keep the quest open until that remaining work is tracked and tested.
AGENTS.md reference: quest/AGENTS.md:L97-L98
Useful? React with 👍 / 👎.
| # The browser check is not `*.test.ts`: plain `bun test` runs lack Playwright Chromium. | ||
| harness: | ||
| ./lib/harness.test.sh | ||
| bun test ./interop/clients/js/harness.browser.ts |
There was a problem hiding this comment.
Install browser prerequisites in the harness recipe
In a clean checkout, just test harness reaches this command without installing workspace dependencies or Playwright Chromium, so it fails to resolve playwright or launch the configured Chromium channel. The preceding Playwright step in .github/workflows/interop.yml happens to prepare CI, but the newly advertised local recipe is not independently runnable; provision the same prerequisites here or invoke an existing setup path before the browser test.
Useful? React with 👍 / 👎.
Problem
Concurrent Nix shells reserved the same relay port because each kept its reservations under its private
TMPDIR, so a secondjust test interopin another worktree saw port 4500 already serving and failed immediately. The browser pause click could hit the canvas after the controls auto-hid. Same-checkout runs also clobbered Go staging output.Approach
/tmp/moq-test-ports-<uid>regardless ofTMPDIR; refuse a reservation root that is a symlink or owned by someone else.just test harnessadds regressions for separate-TMPDIRshells, pinned-port contention, symlink refusal, and canvas-covered controls; the Interop workflow runs it. The browser check isharness.browser.ts, not*.test.ts, since plainbun testruns have no Playwright Chromium.interop-flakesquest.Impact
Alternatives
Binding both listeners to port 0 does not keep UDP QUIC and TCP HTTP/WebSocket on the same port required by the harness URL.
An earlier revision ran two full matrices concurrently in one CI checkout. That clobbers the shared Python venv and maturin output, and doubles load on one runner, so CI runs a single matrix plus the deterministic port regression instead.
Follow-ups
python -> js,go -> js: the Pause control never appears within 30s). Not seen in a single run; worth a quest if it shows up nightly.🤖 Generated with Claude Code
(Written by GPT-6, updated by Claude Opus 5.5)