Skip to content

test(file_index): make index/query/snapshot/watcher tests portable to Windows#505

Merged
Xoshbin merged 2 commits into
Xoshbin:mainfrom
dose-dot-dev:fix/file-index-tests-windows
Jul 14, 2026
Merged

test(file_index): make index/query/snapshot/watcher tests portable to Windows#505
Xoshbin merged 2 commits into
Xoshbin:mainfrom
dose-dot-dev:fix/file-index-tests-windows

Conversation

@dose-dot-dev

Copy link
Copy Markdown
Contributor

Third slice of fixing the 38 Windows test failures that became visible once #499 made cargo test loadable on Windows (context in #498). Test-only — no product change.

Two distinct Windows-only failures

1. Hardcoded unix paths in fixtures (9 tests: 6 index, 2 query, 1 snapshot).
The index materializes paths with MAIN_SEPARATOR and compares that against requested paths, so on Windows a /tmp/rootA root joined with \ (/tmp/rootA\docs) never equals the unix-spelled lookup key — lookups miss and materialize/.path assertions differ by separator. Route fixture paths and their expected strings through a local np() helper (/MAIN_SEPARATOR; no-op on Unix). Slash-token queries are unaffected: tokenize() splits the query on / and matches component names, not the stored separator.

2. The two real-filesystem watcher e2e tests watched the OS temp dir (2 tests).
On Windows that is C:\Users\<u>\AppData\Local\Temp, and AppData/Local is one of DEFAULT_IGNORE_PATTERNS — so build_exclusion_set(&[]) matched the tests' own watched root via **/AppData/Local/** and the coalescer correctly dropped every event (got []). It only passed on Linux because /tmp matches no default pattern. The exclusion logic was working perfectly — the test just put its root inside an excluded directory.

Fix: give those tests an exclusion set scoped to just the pattern under test (node_modules, or none for the rename test), so the temp root isn't collaterally excluded. The default patterns are already covered by the pure-coalescer unit tests.

While there, I also replaced the fixed post-arm sleep with an active arming probe + bounded poll: write a throwaway file until an event lands (proving the backend delivers), then run the real writes and poll up to a budget for the expected events. It's robust to notify's async ReadDirectoryChangesW arming and self-diagnosing — it asserts clearly if a backend ever genuinely fails to deliver, instead of a silent empty list. (That assertion is what pinpointed the AppData/Local collision.)

Why no product change

The index's MAIN_SEPARATOR handling is correct — real OS paths use the native separator on each platform. And excluding AppData/Local is intended; production watches user-configured roots (Documents, projects, …), never the OS temp dir. Both bugs are in the test fixtures.

Verification

  • Windows 11: cargo test file_index fully green (126 tests), including the 11 previously-failing and the two real-FS watcher e2e tests (run repeatedly, stable).
  • Linux (WSL): full cargo test green (known fs_watcher inotify env failure aside); cargo clippy --all-targets -- -D warnings and cargo fmt clean.

🤖 Generated with Claude Code

… Windows

Two distinct Windows-only failures, both test-side:

1) Fixtures hardcoded unix paths ("/tmp/rootA", "/r/..."). The index
materializes paths with MAIN_SEPARATOR and compares that against
requested paths, so on Windows a "/tmp/rootA" root joined with '\'
("/tmp/rootA\docs") never equals the unix-spelled lookup key — breaking
6 index, 2 query, and 1 snapshot test. Route fixture paths and their
expected strings through a local `np()` helper (unix '/' ->
MAIN_SEPARATOR; no-op on Unix). Slash-token queries are unaffected:
tokenize() splits the query on '/' and matches component names, not the
stored separator.

2) The two real-filesystem watcher e2e tests used the OS temp dir as
their watched root. On Windows that is C:\Users\<u>\AppData\Local\Temp,
and "AppData/Local" is one of DEFAULT_IGNORE_PATTERNS — so
build_exclusion_set(&[]) matched the tests' own root via
**/AppData/Local/** and the coalescer correctly dropped every event
("got []"). It only worked on Linux because /tmp matches no default
pattern. Give these tests an exclusion set scoped to just the pattern
under test (node_modules, or none for the rename test) so the temp root
isn't collaterally excluded; the default patterns are covered by the
pure-coalescer unit tests. Also replaced the fixed post-arm sleep with
an active arming probe + bounded poll — robust to notify's async
ReadDirectoryChangesW arming and self-diagnosing if a backend ever
genuinely fails to deliver.

Not a product concern: production watches user-configured roots (never
under AppData/Local), and both fixes are test-only.

Fixes 11 of the 38 Windows failures surfaced by Xoshbin#499 (see Xoshbin#498).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGJAx66y29o33szDvVYTBH

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request improves cross-platform test compatibility (particularly for Windows) by introducing a helper function np to convert Unix-style fixture paths to native platform separators across several test files. It also refactors the file watcher tests to prevent flaky races and avoid default exclusions matching Windows temp directories. The review feedback correctly highlights that using std::path::MAIN_SEPARATOR_STR requires Rust 1.78.0 or later, which may break compilation on older toolchains. It is recommended to use a more compatible character-mapping approach with std::path::MAIN_SEPARATOR instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread asyar-launcher/src-tauri/src/file_index/index.rs
Comment thread asyar-launcher/src-tauri/src/file_index/query.rs
Comment thread asyar-launcher/src-tauri/src/file_index/snapshot.rs
Gemini review: MAIN_SEPARATOR_STR was stabilized in Rust 1.78; map chars
to MAIN_SEPARATOR (stable since 1.0) instead so the helper doesn't assume
a minimum toolchain. (The repo declares no MSRV and CI pins 1.97, so this
is defensive, not a fix — but it's a wash and removes the question.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MGJAx66y29o33szDvVYTBH
@Xoshbin Xoshbin merged commit b88e79f into Xoshbin:main Jul 14, 2026
1 check passed
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.

2 participants