Skip to content

Sanitise Windows paths in IO test fixtures (closes #642)#643

Merged
aallan merged 1 commit into
mainfrom
claude/issue-642-u-escape-fix
May 10, 2026
Merged

Sanitise Windows paths in IO test fixtures (closes #642)#643
aallan merged 1 commit into
mainfrom
claude/issue-642-u-escape-fix

Conversation

@aallan

@aallan aallan commented May 10, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #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 is C:\Users\runner\AppData\Local\Temp\... and 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 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 \U support 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_success and test_io_read_file_roundtrip pass on macOS (existing behaviour preserved)
  • Full pytest suite green (3,782 passed, 14 skipped)
  • Both tests pass on Windows once Add windows-latest to CI test matrix in advisory mode (closes #637) #639's Windows entries pick this up (currently advisory; sub-issue audit verified by checking remaining red Windows checks shrink by 1)

Part of the #637 follow-up cluster

Sibling PRs to land: #640 (/dev/stdin CLI handling), #641 (cp1252 file I/O encoding). Once all three close, the Windows entries can drop continue-on-error and become strict — that lands as part of #641.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Improved file path handling in IO tests by normalising temporary paths to POSIX format, enhancing cross-platform test reliability.

Review Change Stack

…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>
@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 23fbd355-ff7d-4662-8fe5-64a0f9a7bb93

📥 Commits

Reviewing files that changed from the base of the PR and between f148e54 and ae730d7.

📒 Files selected for processing (2)
  • TESTING.md
  • tests/test_codegen.py

📝 Walkthrough

Walkthrough

Two IO file tests now normalise temporary file paths to POSIX form before embedding them into Vera string literals, preventing backslash escape sequences (e.g. \U) from triggering parser errors on Windows. Test metrics table updated to reflect the 9-line net increase in test code.

Changes

Windows path normalisation in IO tests

Layer / File(s) Summary
Path normalisation in test fixtures
tests/test_codegen.py (lines 7755–7765, 7783–7794)
test_io_read_file_success and test_io_read_file_roundtrip now derive vera_path by replacing OS-specific path separators with / before embedding into generated Vera source. Prevents \U and other backslash sequences from triggering Vera parser errors on Windows.
Test metrics documentation
TESTING.md (line 61)
Test Files table entry for test_codegen.py updated to record new total-code line count (17,592), reflecting the 9-line net addition from path normalisation logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

tests

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: normalising Windows paths in IO test fixtures by replacing backslashes with forward slashes before embedding in Vera string literals.
Linked Issues check ✅ Passed The PR fully addresses issue #642: sanitised paths in both test_io_read_file_success and test_io_read_file_roundtrip using forward-slash normalisation to prevent invalid escape sequences.
Out of Scope Changes check ✅ Passed All changes are narrowly scoped to test fixtures: path sanitisation in two IO tests and a metrics update in TESTING.md. No grammar, runtime, or production code modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-642-u-escape-fix

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented May 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.97%. Comparing base (f148e54) to head (ae730d7).

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           
Flag Coverage Δ
javascript 57.36% <ø> (ø)
python 94.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aallan aallan merged commit 9c8dd83 into main May 10, 2026
19 of 22 checks passed
@aallan aallan deleted the claude/issue-642-u-escape-fix branch May 10, 2026 22:23
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test_io_read_file_roundtrip: Windows path embedded in Vera string literal triggers \U escape

1 participant