test: separate "Windows has no such thing" from "we got it wrong" - #95
Open
xizhuomengcontin wants to merge 1 commit into
Open
xizhuomengcontin wants to merge 1 commit into
xizhuomengcontin wants to merge 1 commit into
Conversation
Ten tests failed on Windows, and the suite said nothing about which were real.
That is the actual cost: with ten always red, an eleventh — a genuine
regression — arrives invisible. This sorts them.
One was ours. `api.test.ts` asserted `runDir` contains `.orca/runs`, spelling
the separator itself, so it only ever held on POSIX. The API returns a real
path; the test now builds the expected fragment with `join`. Fixed, not skipped.
Five are things Windows does not have, each skipped with the reason written
where the next person will look:
- `$SHELL` — opencode passes the host's through, and Windows sets none. The
fixture records what the adapter produced on the platform it was captured
on; filtering the key list would make it stop meaning that.
- `HTTPS_PROXY` vs `https_proxy` — one case-insensitive variable on Windows,
so the two spellings cannot be asserted to agree. The child is configured
correctly either way.
- SIGTERM — no signal to forward. Nothing here is broken; there is no
mechanism to test.
- the execute bit — git gives up on it too (`core.fileMode=false`). Only that
one assertion is guarded; the byte-for-byte round trip still runs, and the
consequence is stated in the comment: a run recorded on POSIX and forked on
Windows gets its scripts back without `+x`.
- symlinks — `symlink()` needs Developer Mode or elevation, so the test's own
setup fails and `materialize` is never reached. Unmeasured, not proven.
Four stay red on purpose, and none of them is a platform difference:
- `ca.test.ts` 0600 — the run CA's private key really is readable by every
account on a Windows box. `chmod` there only toggles read-only. This is a
security gap, not a test artifact, and gets its own fix.
- `doctor.test.ts` shell shim — a `.cmd` shim cannot be exec'd by
`CreateProcess`, so shell capture needs a different mechanism on Windows.
A real gap in what orca captures.
- the two fork tests — they hang rather than run slowly; 120s changes
nothing, while `orca compare` against a live gateway finishes in seconds on
this same machine. Something in the fixture path, still undiagnosed.
Silencing those four would have hidden two user-facing gaps and one unexplained
hang. Left red, with the diagnosis in the PR.
Windows: 10 failed before, 4 after — same four on both sides, nothing new.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🐳 OrcaCode Review
✅ No findings — nothing to flag in this PR. Great work!
OrcaCode Review — Route Smarter. Ship Safer. Spend Less.
Engine-reported: 466 calls · 35.5M tokens · 99% cached
❤️ Share · Install OrcaCode Review
Free on GitHub — the review runs on your own OrcaRouter key. If it helped, a shout-out goes a long way.
Share: X · Reddit · LinkedIn
Follow: X · Discord · LinkedIn · OrcaRouter
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ten tests fail on Windows. The suite said nothing about which of them mean anything, and that is
the real cost — with ten permanently red, the eleventh arrives invisible.
This sorts them into three piles and writes the reason for each one where the next person will
look for it.
One was ours
api.test.tsassertedresult.runDircontains.orca/runs— spelling the separator itself, soit only ever held on POSIX. The API returns a real path, and on Windows that is
.orca\runs\run_….Now built with
join. Fixed, not skipped.Five are things Windows does not have
still produces the env it was recorded with$SHELL— opencode passes the host's through, Windows sets nonetrusts the run CA through the child environmentHTTPS_PROXYandhttps_proxyare one case-insensitive variableforwards SIGTERM to the childround-trips a nested tree byte for byteround-trips a symlink as a symlinksymlink()Two are worth spelling out:
The execute bit is guarded inline rather than skipping the test, because the byte-for-byte
round trip either side of it is exactly as meaningful on Windows. Git itself gives up here
(
core.fileMode=false). The consequence is real and the comment says so: a run recorded on POSIXand forked on Windows gets its scripts back without
+x.The symlink test fails in its own setup —
symlink()needs Developer Mode or elevation, somaterializeis never reached. The gap on Windows is therefore unmeasured, not proven. Leftas a red test it read like a restore bug, which it is not.
The
$SHELLone is skipped rather than filtered on purpose: the fixture is a record of what theadapter produced on the platform it was captured on, and filtering the key list would make it
stop meaning that.
Four stay red, deliberately
None of these is a platform difference. Two have since been diagnosed and fixed in follow-ups —
a security hole (#96) and a suite that was calling a real gateway (#97) — which is the argument for
sorting the list rather than making it green.
ca.test.ts— the run CA's private key at 0600. Not a test artifact. On Windowschmodonlytoggles the read-only attribute, so
ca.keyis readable by every account on the box, and that keysigns certificates the agent is told to trust for the duration of the run. Fixed in #96, with
Windows ACLs, rather than riding along here.
doctor.test.ts— the shell shim. A.cmdshim cannot be exec'd byCreateProcess, so thePOSIX interception mechanism does not apply and shell capture needs a different one on Windows.
A real gap in what orca captures, not in how it is tested.
The two fork tests (
compare,e2e) —an unexplained hangdiagnosed, and fixed in#97. They do not hang. Under a full suite they reach the 30s timeout; run alone, each fails in
under two seconds with a 401 from a host the test never named.
replayCommandresolves itsupstream through
readConfig()with no environment argument, so it reads the real~/.config/orca:--upstream-anthropicwas overridden,openaiwas not, and it fell through tomy own configured gateway. Nothing platform-specific, and invisible to CI, which has no config.
This PR leaves them red; #97 turns them green.
Measured
Same machine, same build, quiet:
The four are the same four on both sides — nothing new, nothing masked.
prettier --check,tsc --build --force,scripts/conformance.mjs(63 events, 0 failures) andscripts/fidelity.mjs --check(0 regressions) are all clean.🤖 Generated with Claude Code