test: normalize path separators in 7 cross-platform assertions (#643)#711
Merged
test: normalize path separators in 7 cross-platform assertions (#643)#711
Conversation
The Windows CI job (currently `continue-on-error: true`) had 117 test failures on the latest main (run 25244100712). 7 of those were straightforward path-separator literals in test assertions: production returns a `Path` whose `str()` uses the platform separator (`\\from\\env.db` on Windows), but the tests compared against hardcoded forward-slash literals (`/from/env.db`). Wrap the LHS in `Path(...).as_posix()` so the comparison is portable across POSIX and Windows without changing what the test exercises: - 5 tests in `test_config_overrides.py` (env var precedence over config.json; both scalar and list `memory_dirs` cases plus the PR #247 `.mcp.json` regression and the `config.d` env override). - 1 test in `test_session_summary_rescue.py` (`test_above_threshold_walks_chunk_links_to_source_files`) — set comprehension over chunk source paths. - 1 test in `test_storage.py` (`test_norm_path_osError_fallback_still_normalizes`) — the assertion about NFC normalization of the OSError fallback. The NFC property is still pinned because the RHS goes through `unicodedata.normalize`; `as_posix()` only flattens the platform-dependent separator that the fallback's `str(p)` introduces. Why test-only, not production: `norm_path()` in `storage/sqlite_helpers.py` uses `str(p.resolve())` on purpose. Changing it to `.as_posix()` would alter every existing on-disk path key under the unique-content index and require a migration. Tests should match production behaviour, not the other way around. Out of scope (other Windows CI failure classes — separate PRs): - POSIX file-mode (`stat.S_IMODE`) — issue #637 - HOME monkeypatch reach (`Path.home()` vs `USERPROFILE`) — issue #644 - Path canonicalization in indexing — issue #647 - `.claude/projects` discovery / wizard backslash regex — issue #316 - Symlink + git-status detection - `fcntl` / signal handling (POSIX-only) - `'home'` fs alias / tilde expansion in web fs-list — issue #646 - `config_signature` NTFS mtime resolution — issue #645 Verification: - `uv run pytest packages/memtomem/tests/test_config_overrides.py packages/memtomem/tests/test_session_summary_rescue.py packages/memtomem/tests/test_storage.py -m "not ollama" -q` → 60 passed (macOS regression check). - `uv run ruff check + format --check` clean on the three files. - Windows CI before/after counts to be appended to the PR body once the branch run finishes. Closes #643. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Owner
Author
Windows CI delta (run 25244446944, job 74026052967)
What actually changedAll 7 path-separator tests this PR targets are gone from the failure list ✅:
The net is only −4 because the symlink / git-status failure class is flaky on Windows. Two tests from that class disappeared between runs ( Negative pin (no regression on other OSes)
✅ All required gates green; the Windows job is informational ( |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Closes #643.
Summary
The Windows CI job runs as
informational(continue-on-error: trueat.github/workflows/ci.yml:45). On the latest main run it accumulated 117 test failures, falling into 6 distinct classes. This PR addresses the smallest, lowest-risk class — path-separator literals in test assertions — so the rest can be tackled cleanly in follow-up PRs.7 tests across 3 files compared production output (a
Pathwhosestr()uses the platform separator) against hardcoded forward-slash literals. Wrapping the LHS inPath(...).as_posix()makes the comparison portable without changing what the test actually exercises.Test-only PR. No production source touched (notably
norm_path()instorage/sqlite_helpers.py:24-38is left as-is — itsstr(p.resolve())behaviour is on-disk-key-shaped and changing it would force a migration of every existing chunk row).Cross-check table
Triage was done against
gh run view 25244100712 --log-failed --job=74025071567. Of the 14 candidates flagged by exploration, only the 7 below were genuinely path-separator failures; the others were misclassified and belong to other failure classes (see Out of scope below).test_config_overrides.py::test_env_var_wins_over_config_json_scalar'\\from\\env.db' == '/from/env.db'test_config_overrides.py::test_env_var_wins_over_config_json_list['\\from\\env'] == ['/from/env']test_config_overrides.py::test_env_and_config_coexist_on_different_fieldstest_config_overrides.py::test_regression_pr247_mcp_json_env_blocktest_config_overrides.py::test_config_d_env_wins_over_fragmentstest_session_summary_rescue.py::TestBoostSourcesHelper::test_above_threshold_walks_chunk_links_to_source_files{'\\tmp\\src\\…'} == {'/tmp/src/…'}test_storage.py::TestNormPathUnicode::test_norm_path_osError_fallback_still_normalizes'\\\\tmp\\\\내 드라이브\\\\file.md' == '/tmp/내 드라이브/file.md'The Pattern-1 fix wraps the LHS in
Path(...).as_posix(). For thenorm_pathtest, the comparison is structured so the NFC property is still pinned (RHS goes throughunicodedata.normalize("NFC", nfd));as_posix()only flattens the platform-dependent separator that the OSError fallback'sstr(p)introduces.Verification
uv run pytest packages/memtomem/tests/test_config_overrides.py packages/memtomem/tests/test_session_summary_rescue.py packages/memtomem/tests/test_storage.py -m "not ollama" -q→ 60 passed (macOS regression check).uv run ruff check+uv run ruff format --checkon the three files → clean.Windows CI impact
Measured on this branch's run (
gh run view 25244446944 --log-failed --job=74026052967).The net (−4) understates the path-sep contribution (−7) because the symlink / git-status failure class is flaky on Windows. Two tests from that class disappeared between runs (
test_stale_pin_when_history_rewritten,test_skip_symlinks) and five others appeared (test_behind_state_pin_below_head,test_cli_status_renders_states,test_stale_pin_when_wiki_absent,test_dirty_partial_subdir,test_skip_dotgit_dsstore_pycache) — all in the same flaky class, none caused by this PR. They will be addressed in the dedicated symlink/git-status follow-up.macOS / Ubuntu / lint / typecheck / golden-path / notebooks / CLA all green on this branch.
Out of scope (separate follow-up PRs)
The other 110 Windows failures fall into classes that need different mechanics. Each has its own tracking issue:
stat.S_IMODE, NTFS doesn't honour POSIX permission bits) — issue Windows: replace POSIX file-mode XDG_RUNTIME_DIR tests with NTFS-aware equivalents #637Path.home()usesUSERPROFILEon Windows somonkeypatch.setenv("HOME", …)doesn't take effect) — issue Windows: HOME monkeypatch doesn't reachPath.home()on Windows (uses USERPROFILE) #644kinddetection +memory_dircomparisons) — issue Windows: Path canonicalization in indexing —kinddetection + memory_dir comparisons #647'home'fs alias / tilde expansion in web fs-list — issue Windows:'home'fs alias + tilde expansion in web fs-list endpoint #646config_signatureNTFS mtime resolution / cache — issue Windows: config_signature unchanged after write — NTFS mtime resolution / cache #645fcntl+ signal handling (POSIX-only — needsskipifmarkers)The next-simplest follow-up is the POSIX-only
skipifbatch (file-mode +fcntl+ signal = ~14 tests, single uniform change).Test plan
🤖 Generated with Claude Code