Sanitise Windows paths in IO test fixtures (closes #642)#643
Conversation
…642) `test_io_read_file_success` and `test_io_read_file_roundtrip` embed `tempfile.NamedTemporaryFile().name` / `tempfile.mkdtemp()` directly into a Vera source string via f-string interpolation. On Windows the path looks like `C:\Users\runner\AppData\Local\Temp\...` — Vera's grammar (correctly) rejects `\U` as an invalid escape sequence, producing `[E009] Invalid escape sequence: \U` at parse time. Fix: convert the path to POSIX form (`tmp_path.replace(os.sep, "/")`) before embedding into the Vera source. Windows file APIs accept forward slashes natively, so this works on every platform without runtime changes. Vera's grammar / IO host imports are unchanged; only the test fixtures. No new test needed — the existing tests now exercise both Unix (unchanged behaviour) and Windows (fix landing point) via the matrix once the Windows-CI advisory rollout completes. Closes #642. 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)
📝 WalkthroughWalkthroughTwo IO file tests now normalise temporary file paths to POSIX form before embedding them into Vera string literals, preventing backslash escape sequences (e.g. ChangesWindows path normalisation in IO tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #643 +/- ##
=======================================
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:
|
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>
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>
Summary
Closes #642.
test_io_read_file_successandtest_io_read_file_roundtripembedtempfile.NamedTemporaryFile().name/tempfile.mkdtemp()directly into a Vera source string via f-string interpolation. On Windows the path isC:\Users\runner\AppData\Local\Temp\...and Vera's grammar (correctly) rejects\Uas an invalid escape sequence, producing[E009] Invalid escape sequence: \Uat parse time.Fix
Convert the path to POSIX form via
tmp_path.replace(os.sep, "/")before embedding. Windows file APIs accept forward slashes natively, so this works on every platform without runtime / grammar changes.Why test-only
Vera's escape-set is intentionally minimal — the parser is correct to reject
\U. Adding\Usupport to the grammar would be a language-spec decision, not a Windows-compat fix. The fix belongs in the test fixture, not the language.Test plan
test_io_read_file_successandtest_io_read_file_roundtrippass on macOS (existing behaviour preserved)Part of the #637 follow-up cluster
Sibling PRs to land: #640 (
/dev/stdinCLI handling), #641 (cp1252 file I/O encoding). Once all three close, the Windows entries can dropcontinue-on-errorand become strict — that lands as part of #641.🤖 Generated with Claude Code
Summary by CodeRabbit