ci: run the test suite on Windows for every pull request - #191
Merged
Conversation
Platform-specific failures surfaced only at release time — v0.3.0's tag run died on win-arm64 over a TLS test racing a file lock — and the Windows ACL assertion on the session file had never run in CI at all.
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
1 participant
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.
The suite ran on Linux for every PR and on all six platforms only inside
release.yml's publish matrix. That makes the release the place where you find out a test is platform-specific, which is the worst place for it: v0.3.0's tag run died onwindows-11-arm-win-arm64whenTlsHotReloadTestsraced a file lock, and the tag had to be fixed and re-pushed. Nothing about that failure needed a release to discover.What this adds
A
test (windows)job onwindows-latest, gated by the samechangesfilter as the Linux job, doing restore → build → test.Deliberately thinner than the Linux job: formatting, analyzers and coverage are properties of the code rather than of the operating system, so they stay where they are instead of being paid for twice. The Linux job keeps its name, so any branch protection referencing it is untouched.
What newly gets covered
session.json(JsonSessionStorePermissionsTests) — it asserts a protected DACL whose every entry is the current user, and it can only run on Windows. Shipped in fix(infra): protect session.json with a user-only DACL on Windows #160, verified by hand on one machine, and never executed in CI until now.TlsHotReloadTests, whose Windows file-locking behaviour is what broke the v0.3.0 release.What it does not cover
windows-latestis x64. The v0.3.0 failure was on arm64, and while that particular fault looks like ordinary Windows file locking rather than anything arm-specific, an arm64-only regression would still reach the release gate unnoticed. Adding awindows-11-armrunner to every PR is a bigger bill than this problem has so far justified; macOS is left out for the same reason.