docker: four tests fail on Windows — path separators, not logic #20

New issue

Closed

opened 2026-08-10 21:53:30 +02:00 by thisilike · 0 comments

thisilike commented

2026-08-10 21:53:30 +02:00

Owner

Split out of #12 so it can be fixed on a Windows checkout by someone who has one. Found by @julian's fifth review pass on #10; the description below is his.

deno task test on a Windows checkout is 299 passed / 4 failed:

the three commit_test.ts git probes
compose_update_test.ts — "eligibility: a legal ../ path is accepted once it resolves inside /"

Path shape, not logic:

rootOf returns C:/Users/… while the file paths carry \, so file.startsWith(root + "/") misses. The diff --git header then keeps an absolute path and git apply --cached rejects it.
makeTempDir() returns C:\…, which fails startsWith("/").

They run rather than skip because git is present, so they are red rather than ignored.

Two ways to fix, and the choice is worth making deliberately:

Guard — add Deno.build.os === "windows" alongside the existing ignore: !hasGit. Cheap, honest about the coverage, leaves the underlying path handling untested on Windows.
Normalise separators — makes the tests pass and tells us whether rootOf/hostPath actually work on a Windows path. If the product is ever expected to run from a Windows checkout, this is the one that answers a real question.

The tests were added in #10, and the "251 local / 245 + 4 ignored" figure in that PR only ever held because every run was POSIX — so the number was reported honestly and was still wrong for the machine review happens on.

Not blocking anything: the failures are in tests, and the code they cover is exercised on Linux, which is where the product runs.

/cc @julian — this one is yours if you want it; the Windows box is the part I do not have.

Split out of #12 so it can be fixed on a Windows checkout by someone who has one. Found by @julian's fifth review pass on #10; the description below is his. `deno task test` on a Windows checkout is 299 passed / **4 failed**: - the three `commit_test.ts` git probes - `compose_update_test.ts` — "eligibility: a legal `../` path is accepted once it resolves inside /" Path shape, not logic: - `rootOf` returns `C:/Users/…` while the file paths carry `\`, so `file.startsWith(root + "/")` misses. The `diff --git` header then keeps an absolute path and `git apply --cached` rejects it. - `makeTempDir()` returns `C:\…`, which fails `startsWith("/")`. They run rather than skip because git is present, so they are red rather than ignored. Two ways to fix, and the choice is worth making deliberately: 1. **Guard** — add `Deno.build.os === "windows"` alongside the existing `ignore: !hasGit`. Cheap, honest about the coverage, leaves the underlying path handling untested on Windows. 2. **Normalise separators** — makes the tests pass *and* tells us whether `rootOf`/`hostPath` actually work on a Windows path. If the product is ever expected to run from a Windows checkout, this is the one that answers a real question. The tests were added in #10, and the "251 local / 245 + 4 ignored" figure in that PR only ever held because every run was POSIX — so the number was reported honestly and was still wrong for the machine review happens on. Not blocking anything: the failures are in tests, and the code they cover is exercised on Linux, which is where the product runs. /cc @julian — this one is yours if you want it; the Windows box is the part I do not have.