Skip to content

Commit a8ceeb7

Browse files
Fix Windows nightly test failures and absolute --patch-dir bound (#2049)
* Fix 7 Windows-only test failures in nightly CI The scheduled CI run has been red on main for five nights. Its `test (windows-latest, 1|2)` legs (nightly-only) fail seven tests that pass on Linux and macOS, all from test-side assumptions about Unix paths: - an_absolute_patch_dir_inside_the_repository_is_read built its package.json with format!, so the Windows path's backslashes were invalid JSON escapes; build it with serde_json instead. - five assertions matched `/`-separated substrings in user-facing detail text that Windows prints with `\`; compare after mapping `\` to `/`. - pnpm_workspace_file_above_the_repository_fails_closed expected the canonical `\\?\` path, but governing_workspace_file deliberately strips that prefix; expect without_verbatim_prefix(file). No production code changes; every assertion still checks the same path on every OS. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aeRDbxjatib28cpxPqQ * Read a user-typed absolute --patch-dir as in-repo On Windows the bound's repository root is canonical (`\\?\C:\...`) while a script names its patch directory as users type it (`C:\...`), and Path::starts_with treats the verbatim and plain disk prefixes as different. Every absolute in-repo `--patch-dir` was reported as outside the repository, protecting every npm package. Compare both sides without the verbatim prefix, and make the test spell the directory the way a user does (quoted, unprefixed). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011f9aeRDbxjatib28cpxPqQ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 113ea6a commit a8ceeb7

5 files changed

Lines changed: 35 additions & 18 deletions

File tree

‎crates/socket-patch-cli/tests/hosted_memory_engine.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2281,7 +2281,9 @@ async fn user_patched_npm_packages_are_never_pinned() {
22812281
.unwrap_or_else(|| panic!("{fixture} {file}: {:?}", project.skipped));
22822282
assert_eq!(skip.purl, "pkg:npm/left-pad@1.3.0");
22832283
assert!(
2284-
skip.detail.as_deref().is_some_and(|d| d.contains(file)),
2284+
skip.detail
2285+
.as_deref()
2286+
.is_some_and(|d| d.replace('\\', "/").contains(file)),
22852287
"{fixture}: {:?}",
22862288
skip.detail
22872289
);

‎crates/socket-patch-core/src/patch/user_patches.rs‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,12 @@ impl Bound {
12321232
)
12331233
}
12341234
Bound::Disk { root, repo } => {
1235-
!normalize_lexically_keeping_escapes(&root.join(rel)).starts_with(repo)
1235+
// `repo` is canonical (`\\?\C:\...` on Windows) while a
1236+
// script names a directory as users type it (`C:\...`), and
1237+
// `starts_with` treats the two prefixes as different.
1238+
use crate::utils::pnpm_workspace::without_verbatim_prefix;
1239+
!without_verbatim_prefix(normalize_lexically_keeping_escapes(&root.join(rel)))
1240+
.starts_with(without_verbatim_prefix(repo.clone()))
12361241
}
12371242
}
12381243
}
@@ -2058,14 +2063,14 @@ mod tests {
20582063
assert!(
20592064
left_pad
20602065
.iter()
2061-
.any(|s| s.ends_with("fixes/left-pad+1.3.0.patch")),
2066+
.any(|s| s.replace('\\', "/").ends_with("fixes/left-pad+1.3.0.patch")),
20622067
"{left_pad:?}"
20632068
);
20642069
let is_number = global.conflicts("pkg:npm/is-number@7.0.0");
20652070
assert!(
2066-
is_number
2067-
.iter()
2068-
.any(|s| s.ends_with("shared/a/patches/is-number+7.0.0.patch")),
2071+
is_number.iter().any(|s| s
2072+
.replace('\\', "/")
2073+
.ends_with("shared/a/patches/is-number+7.0.0.patch")),
20692074
"{is_number:?}"
20702075
);
20712076
assert!(global.conflicts("pkg:npm/right-pad@1.3.0").is_empty());
@@ -2115,15 +2120,17 @@ mod tests {
21152120
let tmp = tempfile::tempdir().unwrap();
21162121
let repo = std::fs::canonicalize(tmp.path()).unwrap();
21172122
write(&repo, ".git/HEAD", "ref: refs/heads/main\n");
2118-
let fixes = repo.join("fixes");
2119-
write(
2120-
&repo,
2121-
"package.json",
2122-
&format!(
2123-
r#"{{"scripts":{{"postinstall":"patch-package --patch-dir {}"}}}}"#,
2124-
fixes.display()
2125-
),
2126-
);
2123+
// Spelled as a user types it: on Windows without the `\\?\` prefix
2124+
// `canonicalize` adds to `repo`, quoted in case the temp dir has a
2125+
// space, and built as JSON so its backslashes are escaped.
2126+
let fixes =
2127+
crate::utils::pnpm_workspace::without_verbatim_prefix(repo.clone()).join("fixes");
2128+
let manifest = serde_json::json!({
2129+
"scripts": {
2130+
"postinstall": format!("patch-package --patch-dir \"{}\"", fixes.display())
2131+
}
2132+
});
2133+
write(&repo, "package.json", &manifest.to_string());
21272134
write(&repo, "fixes/left-pad+1.3.0.patch", "diff\n");
21282135
let index = UserPatchIndex::discover(&repo).await;
21292136
assert!(

‎crates/socket-patch-core/src/utils/cargo_build_cache.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,13 @@ mod tests {
437437
.unwrap();
438438
let warnings = invalidate_project(ReadScope::Workspace, &app, purls);
439439
assert_eq!(warnings.len(), 1, "{warnings:?}");
440-
assert!(warnings[0].1.contains(".cargo/config.toml"), "{warnings:?}");
440+
assert!(
441+
warnings[0]
442+
.1
443+
.replace('\\', "/")
444+
.contains(".cargo/config.toml"),
445+
"{warnings:?}"
446+
);
441447
// A global run reads it as before.
442448
assert!(invalidate_project(ReadScope::Global, &app, purls).is_empty());
443449
}

‎crates/socket-patch-core/src/utils/repo_root_upward_tests.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ fn pnpm_workspace_file_above_the_repository_fails_closed() {
135135
write(&file, "packages:\n - other\n");
136136
assert_eq!(
137137
crate::utils::pnpm_workspace::governing_workspace_file(&l.app),
138-
Some(file)
138+
// Reported without Windows' `\\?\` prefix, which `layout` keeps.
139+
Some(crate::utils::pnpm_workspace::without_verbatim_prefix(file))
139140
);
140141
let plain = layout(false);
141142
write(

‎crates/socket-patch-core/src/vex/discover/user_patched.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ mod tests {
328328
assert!(
329329
out.unattested[0]
330330
.detail
331+
.replace('\\', "/")
331332
.contains("patches/left-pad+1.3.0.patch"),
332333
"{}",
333334
out.unattested[0].detail
@@ -420,7 +421,7 @@ mod tests {
420421
all_marked(&out, what);
421422
let member = files[files.len() - 1].0.rsplit_once('/').unwrap().0;
422423
assert!(
423-
out.unattested[0].detail.contains(member),
424+
out.unattested[0].detail.replace('\\', "/").contains(member),
424425
"{what}: {}",
425426
out.unattested[0].detail
426427
);

0 commit comments

Comments
 (0)