Skip to content

fix(test): unbreak Windows runtime tests#267

Merged
emal-avala merged 1 commit into
mainfrom
fix/windows-runtime-tests
May 4, 2026
Merged

fix(test): unbreak Windows runtime tests#267
emal-avala merged 1 commit into
mainfrom
fix/windows-runtime-tests

Conversation

@emal-avala
Copy link
Copy Markdown
Member

Summary

Five tests compiled on Windows but failed at runtime on main. Each had a distinct cross-platform bug; this PR fixes them at the underlying cause rather than disabling.

  • services::profiles::tests::load_profile_migrates_legacy_token_field_and_rewrites_on_disk — replace hand-rolled env stashing with the shared EnvGuard from crates/lib/src/test_support.rs so the test serialises against every other env-mutating test.
  • tools::bash::sed_validation::tests::relative_paths_are_resolved_against_cwd — cfg-split the deny-substring + cwd path so the test matches the platform's native separator (/ on POSIX, \ on Windows). The expected PathBuf derives from cwd.join(".git/HEAD") so it stays separator-agnostic.
  • tools::brief::tests::validate_attachments_rejects_outside_cwd_and_home/etc/hostname doesn't exist on Windows (existence check fires first, masking the boundary check). A TempDir-based outside file would also lose to the dirs::home_dir() containment check on Windows because tempdirs sit under %USERPROFILE%\AppData\Local\Temp and dirs::home_dir() ignores HOME there. Use /etc/hostname on POSIX and C:\Windows\System32\drivers\etc\hosts on Windows.
  • tools::config_tool::tests::set_user_scope_writes_to_xdg_config_home + tools::config_tool::tests::concurrent_set_calls_persist_both_keys — both tests assumed XDG_CONFIG_HOME would redirect dirs::config_dir() on every platform. It doesn't on Windows (dirs reads %APPDATA% via SHGetKnownFolderPath). Introduce crate::config::agent_config_dir() that consults XDG_CONFIG_HOME first on every platform, route every dirs::config_dir().map(|d| d.join(\"agent-code\")) call site through it, and the tests' single env override becomes hermetic on Linux, macOS, and Windows alike.

The new agent_config_dir() helper supersedes the in-tree pattern dirs::config_dir().map(|d| d.join(\"agent-code\")) everywhere it appeared (15 call sites in crates/lib and crates/cli); the only remaining dirs::config_dir() call is inside the helper itself. The schedule e2e tests in crates/cli/tests/schedule.rs that currently #[cfg_attr(target_os = \"windows\", ignore)] for this exact reason are intentionally left alone — re-enabling them is out of scope for this PR.

Test plan

  • cargo fmt --all -- --check clean
  • cargo check --all-targets clean
  • cargo clippy --all-targets -- -D warnings clean
  • cargo test --all-targets passes on Linux (modulo pre-existing bwrap_* sandbox failures from missing user-namespace permissions)
  • CI green on the windows-latest runner (this is the canonical signal for these fixes)

Five tests compiled on Windows but failed at runtime — fix each at the
underlying cause rather than disabling.

- profiles loader: stop poking process-global env vars by hand; use
  the shared `EnvGuard` from `test_support` so the test serialises
  against every other env-mutating test in the crate.
- sed validation: cfg-split the deny-substring and cwd path so the
  test matches the platform's native separator (`/` on POSIX, `\`
  on Windows). The expected `PathBuf` derives from the cwd join so
  it stays separator-agnostic.
- brief attachments: a `TempDir`-based "outside" file lives under
  `%USERPROFILE%\AppData\Local\Temp` on Windows, so the home-prefix
  check accepts it (HOME-env override doesn't reach `dirs::home_dir()`
  on Windows). Use `/etc/hostname` on POSIX and a stable system path
  outside the user profile on Windows.
- config_tool (both tests): introduce `agent_config_dir()` in
  `crates/lib/src/config/mod.rs` that honors `XDG_CONFIG_HOME` on
  every platform (the base `dirs::config_dir()` ignores it on
  Windows). Route every existing `dirs::config_dir().map(|d|
  d.join("agent-code"))` call site through the new helper so a
  single `XDG_CONFIG_HOME` override is hermetic on Linux, macOS,
  and Windows alike.
@emal-avala emal-avala merged commit cd30f0e into main May 4, 2026
14 checks passed
@emal-avala emal-avala deleted the fix/windows-runtime-tests branch May 4, 2026 06:33
@emal-avala emal-avala mentioned this pull request May 4, 2026
12 tasks
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.

1 participant