From 34f514fe1681725f09b377e7904d4a470d30bb3a Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Thu, 3 Sep 2026 14:17:34 -0700 Subject: [PATCH] windows: hook tests pass on Windows Same verbatim-prefix strip on the test temp roots, plus expectations derived from the helpers the product uses: cache keys and scan targets from `jsp::join`, the config path in an admin message from the same relative form `path.relative` renders, and the footer hints from `quote_command_arg`, which deliberately switches to the double-quoted Windows form (#476 / #533). The env lock no longer poisons the sibling tests when one of them fails. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY --- crates/hook/tests/cache_root_tests.rs | 16 +++++++---- crates/hook/tests/hook_tests.rs | 41 ++++++++++++++++++++------- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/crates/hook/tests/cache_root_tests.rs b/crates/hook/tests/cache_root_tests.rs index 090e811c0..8f14dba50 100644 --- a/crates/hook/tests/cache_root_tests.rs +++ b/crates/hook/tests/cache_root_tests.rs @@ -52,7 +52,11 @@ impl Tmp { std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos() )); std::fs::create_dir_all(&base).unwrap(); - Tmp(std::fs::canonicalize(&base).unwrap()) + // Like Node's `realpathSync`: no `\\?\` verbatim prefix on Windows, so the + // paths the hook joins under this root resolve (the kernel takes a + // verbatim path literally and rejects a forward slash). + let real = std::fs::canonicalize(&base).unwrap().to_string_lossy().into_owned(); + Tmp(PathBuf::from(real.strip_prefix(r"\\?\").unwrap_or(&real))) } fn path(&self) -> String { self.0.to_string_lossy().into_owned() @@ -87,7 +91,7 @@ const CLEAN_CSS: &str = ".card { color: #333; }\n"; #[test] fn state_relocates_and_slug_normalizes() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let root = Tmp::new(); let _g = EnvGuard::set(&[("IMPECCABLE_CACHE_ROOT", Some(&root.path()))]); @@ -122,7 +126,7 @@ fn stock_cache_path() -> String { #[test] fn root_value_normalization_and_opt_out() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let root = Tmp::new(); // Stray whitespace in env files trims away. let padded = format!(" {} ", root.path()); @@ -149,7 +153,7 @@ fn root_value_normalization_and_opt_out() { #[cfg(unix)] #[test] fn tilde_expands_against_homedir_or_rejects() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let home = Tmp::new(); let explicit = { let joined = format!("{}/caches", home.path()); @@ -172,7 +176,7 @@ fn tilde_expands_against_homedir_or_rejects() { #[test] fn run_hook_persists_and_dedupes_through_the_redirect() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let root = Tmp::new(); let project = Tmp::new(); let _g = EnvGuard::set(&[("IMPECCABLE_CACHE_ROOT", Some(&root.path()))]); @@ -202,7 +206,7 @@ fn run_hook_persists_and_dedupes_through_the_redirect() { #[test] fn no_footprint_noop_gate_holds_under_redirect() { - let _l = ENV_LOCK.lock().unwrap(); + let _l = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let root = Tmp::new(); let project = Tmp::new(); let _g = EnvGuard::set(&[("IMPECCABLE_CACHE_ROOT", Some(&root.path()))]); diff --git a/crates/hook/tests/hook_tests.rs b/crates/hook/tests/hook_tests.rs index 438c6434e..0c2e38e47 100644 --- a/crates/hook/tests/hook_tests.rs +++ b/crates/hook/tests/hook_tests.rs @@ -10,7 +10,7 @@ use std::collections::HashMap; use std::path::PathBuf; -use impeccable_common::Io; +use impeccable_common::{jsp, Io}; use impeccable_core::findings::{finding, Finding}; use impeccable_detect::MissingHtmlEngine; use impeccable_hook::hook_lib::*; @@ -37,7 +37,11 @@ impl Tmp { )); std::fs::create_dir_all(&base).unwrap(); // Canonical path so the JS-style path helpers and the fs agree on macOS. - Tmp(std::fs::canonicalize(&base).unwrap()) + // Like Node's `realpathSync`, without the `\\?\` verbatim prefix Windows + // adds: the kernel takes a verbatim path literally, so a `/` joined + // under it would not resolve. + let real = std::fs::canonicalize(&base).unwrap().to_string_lossy().into_owned(); + Tmp(PathBuf::from(real.strip_prefix(r"\\?\").unwrap_or(&real))) } fn path(&self) -> String { self.0.to_string_lossy().into_owned() @@ -61,6 +65,13 @@ impl Drop for Tmp { } } +/// The shared config path as the admin verbs print it: relative to the +/// project, in the host's path form (backslashes on Windows, as `path.relative` +/// renders it there). +fn shared_config_rel() -> String { + jsp::join(&[".impeccable", "config.json"]) +} + fn rt_with(cwd: &str, env: HashMap) -> Runtime<'static> { Runtime::new( cwd.to_string(), @@ -461,7 +472,12 @@ fn render_template_caps_and_footers() { )); assert!(text.contains("... and 7 more (see /impeccable audit).")); assert_eq!(text.lines().filter(|l| l.starts_with("- L")).count(), 5); - assert!(text.contains("Run `'/opt/bin/impeccable' hooks ignore-value \"\" --reason \"\"`")); + // The self-command is quoted in the host's shell form (#476 / #533): + // single quotes under sh, double quotes on Windows. + let self_cmd = quote_command_arg("/opt/bin/impeccable", cfg!(windows)); + assert!(text.contains(&format!( + "Run `{self_cmd} hooks ignore-value \"\" --reason \"\"`" + ))); assert!(text.contains("Full suppression ladder: /impeccable hooks.")); let short = render_template( &r, @@ -545,7 +561,10 @@ fn render_template_dedupes_descriptions_and_quotes_hints() { &c, &opts("/x"), ); - assert!(hostile.contains("ignore-value overused-font '$(touch pwned)'")); + assert!(hostile.contains(&format!( + "ignore-value overused-font {}", + quote_command_arg("$(touch pwned)", cfg!(windows)) + ))); let no_hint = { let mut x = f("side-tab", 1.0, "Side tab", "d", "s"); x.extras.insert("ignoreValue".into(), json!("Inter")); @@ -686,7 +705,9 @@ fn harness_detection_and_github_normalization() { assert_eq!(resolve_harness(&forced, Some(&gh)), "codex"); assert_eq!( parse_apply_patch_paths(&r, "*** Begin Patch\n*** Update File: a.css\r\n*** Add File: /abs/b.css\n*** Delete File: c.css\n", "/p"), - vec!["/p/a.css", "/abs/b.css"] + // A relative patch path is resolved against the cwd with the host's + // path semantics; an already-absolute one is passed through. + vec![jsp::join(&["/p", "a.css"]), "/abs/b.css".to_string()] ); assert_eq!( payload("t", "Stop", "claude"), @@ -723,7 +744,7 @@ fn expand_scan_targets_follows_styles() { "src/styles.css", "src/index.sass", ] { - assert!(out.contains(&format!("{cwd}/{name}")), "{name} in {out:?}"); + assert!(out.contains(&jsp::join(&[&cwd, name])), "{name} in {out:?}"); } let rel = expand_scan_targets(&r, &["src/App.jsx".into()], &cwd); assert_eq!( @@ -1018,7 +1039,7 @@ fn run_hook_co_located_styles_and_tiering_config() { json!(1) ); assert_eq!( - cache["sessions"]["s1"]["files"][format!("{cwd}/src/styles.css")]["editCount"], + cache["sessions"]["s1"]["files"][jsp::join(&[&cwd, "src/styles.css"])]["editCount"], json!(0), "co-scanned styles do not bump" ); @@ -1252,7 +1273,7 @@ fn before_edit_denies_shell_and_edit_shapes() { assert!(last.contains("This is the 7th repeated denial for the same file and finding signature, so Impeccable is allowing this write to avoid a loop.")); let cache = read_cache(&cwd); assert_eq!( - cache["sessions"]["cv1"]["files"][format!("{cwd}/src/new.css")]["cursorDenials"] + cache["sessions"]["cv1"]["files"][jsp::join(&[&cwd, "src/new.css"])]["cursorDenials"] ["gradient-text:1"], json!(7) ); @@ -1281,7 +1302,7 @@ fn admin_ignore_value_scoping_and_idempotency() { assert_eq!(code, 0); assert_eq!( out, - "Added overused-font=inter to shared detector.ignoreValues (.impeccable/config.json).\n" + format!("Added overused-font=inter to shared detector.ignoreValues ({}).\n", shared_config_rel()) ); assert!(!t.exists(".impeccable/config.local.json")); let cfg: Value = serde_json::from_str(&t.read(".impeccable/config.json")).unwrap(); @@ -1381,7 +1402,7 @@ fn admin_on_off_preserve_sibling_hook_fields() { let (out, _, _) = admin_run(&r, &["off"]); assert_eq!( out, - "Design hook disabled for this project (wrote .impeccable/config.json).\n" + format!("Design hook disabled for this project (wrote {}).\n", shared_config_rel()) ); let cfg: Value = serde_json::from_str(&t.read(".impeccable/config.json")).unwrap(); assert_eq!(cfg["hook"]["quiet"], json!(true));