Add windows-latest to CI test matrix in advisory mode (closes #637)#639
Merged
Conversation
Single-line addition to `.github/workflows/ci.yml`'s test matrix
that mirrors the existing per-version coverage onto Windows.
Before: `{ubuntu, macos} × {3.11, 3.12, 3.13}` = 6 entries
After: `{ubuntu, macos, windows} × {3.11, 3.12, 3.13}` = 9 entries
Closes the OS-coverage gap flagged after PR #631's CI cycle: a
Python 3.11 syntax bug was caught by the existing matrix's 3.11
entries, but a hypothetical Windows-only-at-3.11 bug would have
slipped through. Windows-specific bug surfaces this catches:
- Path separator handling (raw string concat in module resolution
/ source-file fixtures)
- Line ending differences (CRLF vs LF in conformance fixtures,
lark grammar handling)
- Locale / encoding (cp1252 default in some Windows Python
contexts)
- Wasmtime Windows-specific quirks (DLL loading, ctypes ABI)
- Subprocess invocations (`.exe` extension, PATH handling, shell
dispatch in `tests/test_cli.py`)
- Browser-target tests (Playwright Windows binary differences)
`windows-latest` chosen for symmetry with existing `*-latest`
convention; resolves to Windows Server 2022 today, will roll to
Server 2025 on GitHub's published schedule.
The "+ coverage" gating cell at line 18 / 41 / 45-46 stays
constrained to `ubuntu-latest, 3.12` — coverage remains single-
machine, single-version (no redundant uploads).
Closes #637.
Co-Authored-By: Claude <noreply@anthropic.invalid>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe CI ChangesCI Matrix Expansion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #639 +/- ##
=======================================
Coverage 90.97% 90.97%
=======================================
Files 59 59
Lines 23134 23134
Branches 259 259
=======================================
Hits 21047 21047
Misses 2080 2080
Partials 7 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This was referenced May 10, 2026
Closed
First-run Windows CI surfaced 17 test failures across 4 categories
— all pre-existing Windows-compatibility bugs that the matrix
change is the messenger for, not the cause of.
Per the agreed test plan ("don't conflate matrix expansion with
bug fixing"), the matrix entries land in advisory mode while
each bug is fixed in its own focused PR:
- #640: Vera CLI's /dev/stdin Unix-only — 6 test failures
in tests/test_cli.py::TestStdinInput::*
- #641: Default cp1252 file I/O encoding — ~9 failures with
UnicodeEncodeError on '→' / UnicodeDecodeError on 0x97
(em-dash in cp1252)
- #642: test_io_read_file_roundtrip embeds Windows path
(C:\\Users\\...) into a Vera string literal; grammar
correctly rejects \\U as invalid escape
Mechanism: continue-on-error: ${{ matrix.os == 'windows-latest' }}
on the test job. Windows failures show as advisory (yellow) in
the checks UI rather than blocking merges (red). Existing
Ubuntu/macOS entries stay strict.
When #640, #641, #642 land, drop the continue-on-error line and
Windows becomes a strict gate alongside the other 6 entries.
KNOWN_ISSUES.md updated:
- #637 entry rewritten to note the advisory state + the three
blocking sub-issues
- #640, #641, #642 added under "Test coverage gaps"
Co-Authored-By: Claude <noreply@anthropic.invalid>
This was referenced May 10, 2026
aallan
added a commit
that referenced
this pull request
May 10, 2026
Closes the third sub-issue from PR #639's matrix-expansion follow-up cluster (#641 cp1252 file I/O encoding) and flips the Windows matrix entries from advisory to strict. This is the "now works on Windows" release. Set PYTHONUTF8=1 in the CI test job environment so Python's text-mode open() defaults to UTF-8 regardless of locale (PEP 540). Covers all CI-side encoding failures with a single config change rather than auditing ~30 individual call sites. Closes ~9 failing tests across test_codegen.py, test_codegen_monomorphize.py, test_codegen_closures.py, test_html.py. Also added explicit encoding='utf-8' to vera/parser.py's grammar load — the load-bearing site that runs on every parse, so users on Windows without PYTHONUTF8=1 in their shell still get correct grammar loading (defense-in-depth). Filed #645 for the broader audit (every text-mode open() / read_text() / write_text() call site for explicit encoding) as the durable fix. Drop continue-on-error advisory mode PR #639 added Windows entries with continue-on-error: ${{ matrix.os == 'windows-latest' }} so failures didn't block merges while #640, sub-issues now closed (PR for #642 sanitises Windows paths in test fixtures; PR for #640 reads sys.stdin cross-platform; this PR closes #641). The continue-on-error line is removed in this release; the matrix is now fully strict across all 9 entries (ubuntu / macos / windows × 3.11 / 3.12 / 3.13). Version bump 0.0.142 → 0.0.143 This is the Windows-compatibility milestone — the project went from "untested on Windows" to "Windows is a strict gate" in three sequential PRs (#643 / #644 / this). Marking with a minor version bump and "now works on Windows" framing in CHANGELOG. Co-Authored-By: Claude <noreply@anthropic.invalid>
aallan
added a commit
that referenced
this pull request
May 10, 2026
The post-#637 Windows CI rollout (PRs #639/#643/#644/#646) hit three test-fixture footguns — `tempfile.NamedTemporaryFile`'s delete-while-open behaviour, Windows paths embedded in Vera string literals tripping the `\U` escape, and cp1252 default file I/O encoding. Each had a portable workaround. Added a "Test Fixture Conventions" section to TESTING.md (immediately before "Adding Tests") with the three rules + their canonical wrong/right code snippets. Each rule cites the test that surfaced the bug + the PR that fixed it. Added a brief "Cross-platform pitfalls" section to CLAUDE.md's project-orientation footer that points at the TESTING.md section. The CLAUDE.md entry is a one-bullet pointer per rule, not a full repeat — keeping the substantive content in TESTING.md as the single source of truth. These notes will save the next contributor (or agent) from re-discovering the same footguns. Co-Authored-By: Claude <noreply@anthropic.invalid>
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.
Summary
Closes #637.
Two-line addition to
.github/workflows/ci.ymladding threewindows-latestmatrix entries — but in advisory mode (continue-on-error: truefor Windows only) because three pre-existing Windows-compatibility bugs surfaced on first CI run. The advisory mode lets us merge the matrix expansion now (so Windows visibility lands) while the underlying bugs are fixed in their own focused PRs.Matrix change:
{ubuntu, macos} × {3.11, 3.12, 3.13}= 6 entries{ubuntu, macos, windows} × {3.11, 3.12, 3.13}= 9 entries (Windows advisory)Why advisory mode
First CI run with Windows entries surfaced 17 test failures across 4 categories — all genuinely Windows-specific bugs that pre-date this PR. Per the test plan we agreed (don't conflate matrix expansion with bug fixing), each is now a focused sub-issue:
tests/test_cli.py::TestStdinInput::*— Vera CLI's/dev/stdinpath is Unix-only→and—characters trip'charmap' codecerrors in file I/O without explicitencoding='utf-8'test_io_read_file_roundtripembeds a Windows path with\Uinto a Vera string literal; grammar correctly rejectsEach sub-issue has its own acceptance criteria. When all three land, drop the
continue-on-errorand Windows entries become strict gates again.Why merge advisory rather than wait
ubuntu-latest, 3.12(single-machine, single-version, no redundant uploads).Acceptance criteria
windows-latestcontinue-on-error: ${{ matrix.os == 'windows-latest' }}continue-on-errorline in a follow-upChoice of
windows-latestChosen for symmetry with the existing
*-latestconvention; resolves to Windows Server 2022 today, will roll to Server 2025 on GitHub's published schedule. Server vs consumer Windows differences are negligible for compiler / runtime work — both share the NT kernel, Python distribution, and line-ending / PATH conventions.🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Documentation