Skip to content

Commit e89dde2

Browse files
committed
Apply /code-review findings on apply fixtures
Gem suites that pin --cwd run their child from the runner's directory again, so a crawler reading the process cwd still fails them. apply_invariants runs apply through apply_fixture::run_apply, the --cwd remedy test stages its blob with stage_blob, and the gem --check fixture derives its specifications dir from install_gem's path. bun_global_store and lockfile_only_skip build their records through one index_js_patches helper. apply_network passes bytes to one_file instead of precomputed hashes. Stale module docs updated. Assisted-by: Claude Code:claude-opus-5-5
1 parent ae3de22 commit e89dde2

9 files changed

Lines changed: 70 additions & 88 deletions

‎crates/socket-patch-cli/tests/apply/apply_fixture.rs‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,22 @@ pub fn hashed_file(path: &str, before_hash: &str, after_hash: &str) -> Value {
5353
json!({ path: { "beforeHash": before_hash, "afterHash": after_hash } })
5454
}
5555

56+
/// One `package/index.js` record from `before` to `after` per purl, the
57+
/// i-th with uuid `<prefix>-0000-4000-8000-<i, 12 digits>`.
58+
pub fn index_js_patches(purls: &[&str], uuid_prefix: &str, before: &[u8], after: &[u8]) -> Value {
59+
let files = one_file("package/index.js", before, after);
60+
Value::Object(
61+
purls
62+
.iter()
63+
.enumerate()
64+
.map(|(i, purl)| {
65+
let uuid = format!("{uuid_prefix}-0000-4000-8000-{i:012}");
66+
(purl.to_string(), patch_record(&uuid, files.clone()))
67+
})
68+
.collect(),
69+
)
70+
}
71+
5672
/// Write `.socket/manifest.json`, creating `.socket/blobs/` alongside as
5773
/// a real project has it.
5874
pub fn write_manifest(root: &Path, patches: Value) {
@@ -253,7 +269,9 @@ pub fn write_sourceless_npm_patches(root: &Path, n: u32) {
253269
// ═══════════ gem release-variant mismatch fixtures ═══════════
254270
//
255271
// Shared by `cli_gem_variant_mismatch_policy.rs` (the `--json`
256-
// contracts) and `apply_render.rs` (the human output).
272+
// contracts) and `apply_render.rs` (the human output). `run_gem` also
273+
// runs the gem_stale_bundler_plugin, in_process_gem_config_warning and
274+
// in_process_gem_fallback_home children.
257275

258276
/// The rack singleton's on-disk bytes after a local edit: they match
259277
/// neither the record's beforeHash nor its afterHash.
@@ -265,7 +283,9 @@ pub const NOKOGIRI_DARWIN_MARKER: &[u8] = b"\n# DARWIN-MARKER\n";
265283

266284
/// Run `socket-patch <args>` in `cwd` like `common::run_with_env`, with
267285
/// the ambient `BUNDLE_*` also scrubbed so only the argv and `env` decide
268-
/// what Bundler state the gem crawler reads.
286+
/// what Bundler state the gem crawler reads. Suites that pin `--cwd`
287+
/// pass `Path::new(".")` so the child stays in the runner's directory and
288+
/// a crawler reading the process cwd instead of `--cwd` finds nothing.
269289
pub fn run_gem(cwd: &Path, args: &[&str], env: &[(&str, &str)]) -> (i32, String, String) {
270290
let mut cmd = hermetic_command(&binary());
271291
cmd.args(args).current_dir(cwd);

‎crates/socket-patch-cli/tests/apply/apply_invariants.rs‎

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use std::path::Path;
2020
use serde_json::json;
2121

2222
use super::apply_fixture::{
23-
hashed_file, install_npm_pkg, one_file, patch_record, stage_blob, write_manifest,
23+
hashed_file, install_npm_pkg, one_file, patch_record, run_apply, stage_blob, write_manifest,
2424
write_root_package_json,
2525
};
2626
use crate::common::{binary, git_sha256, hermetic_command};
@@ -44,20 +44,6 @@ fn write_project(root: &Path) {
4444
stage_blob(root, "sentinel", b"do not modify me");
4545
}
4646

47-
fn run_apply(cwd: &Path, extra: &[&str]) -> (i32, String) {
48-
let mut args = vec!["apply", "--json"];
49-
args.extend_from_slice(extra);
50-
let out = hermetic_command(&binary())
51-
.args(&args)
52-
.current_dir(cwd)
53-
.output()
54-
.expect("run socket-patch");
55-
(
56-
out.status.code().unwrap_or(-1),
57-
String::from_utf8_lossy(&out.stdout).to_string(),
58-
)
59-
}
60-
6147
/// Every counter in the envelope's `summary` block must be exactly 0.
6248
/// We enumerate the keys explicitly (rather than "applied == 0") so a
6349
/// regression that started reporting work on these no-op paths — e.g. a
@@ -119,9 +105,10 @@ fn offline_ecosystems_filter_ignores_out_of_scope_missing_source() {
119105
let tmp = tempfile::tempdir().expect("tempdir");
120106
write_mixed_scope_project(tmp.path());
121107

122-
let (code, stdout) = run_apply(
108+
let (code, stdout, _) = run_apply(
123109
tmp.path(),
124-
&["--offline", "--silent", "--ecosystems", "npm"],
110+
&["--json", "--offline", "--silent", "--ecosystems", "npm"],
111+
&[],
125112
);
126113
let v: serde_json::Value =
127114
serde_json::from_str(&stdout).expect("apply --json must emit valid JSON");
@@ -156,7 +143,7 @@ fn offline_ecosystems_filter_ignores_out_of_scope_missing_source() {
156143
// still fire — the guard is scoped, not disabled.
157144
let tmp2 = tempfile::tempdir().expect("tempdir");
158145
write_mixed_scope_project(tmp2.path());
159-
let (code2, stdout2) = run_apply(tmp2.path(), &["--offline", "--silent"]);
146+
let (code2, stdout2, _) = run_apply(tmp2.path(), &["--json", "--offline", "--silent"], &[]);
160147
assert_eq!(
161148
code2, 1,
162149
"with no ecosystem filter the sourceless pypi patch is in scope and \
@@ -172,7 +159,7 @@ fn offline_with_missing_source_emits_partial_failure() {
172159
let tmp = tempfile::tempdir().expect("tempdir");
173160
write_project(tmp.path());
174161

175-
let (code, stdout) = run_apply(tmp.path(), &["--offline", "--silent"]);
162+
let (code, stdout, _) = run_apply(tmp.path(), &["--json", "--offline", "--silent"], &[]);
176163

177164
// Exit code 1 is contract: any patch without a usable source under
178165
// `--offline` flips the run to partialFailure.
@@ -215,7 +202,7 @@ fn apply_does_not_mutate_socket_dir_offline() {
215202

216203
let socket = tmp.path().join(".socket");
217204
let before = snapshot_dir(&socket);
218-
let (code, stdout) = run_apply(tmp.path(), &["--offline", "--silent"]);
205+
let (code, stdout, _) = run_apply(tmp.path(), &["--json", "--offline", "--silent"], &[]);
219206
let after = snapshot_dir(&socket);
220207

221208
// The run must have actually taken the failure path we care about —
@@ -243,7 +230,7 @@ fn apply_does_not_mutate_socket_dir_when_no_packages_match() {
243230

244231
let socket = tmp.path().join(".socket");
245232
let before = snapshot_dir(&socket);
246-
let (code, stdout) = run_apply(tmp.path(), &["--silent"]);
233+
let (code, stdout, _) = run_apply(tmp.path(), &["--json", "--silent"], &[]);
247234
let after = snapshot_dir(&socket);
248235

249236
// Pin the bail itself, or a run that died early would leave
@@ -290,7 +277,7 @@ fn apply_does_not_mutate_socket_dir_when_no_packages_match() {
290277
fn apply_with_no_socket_dir_emits_no_manifest_envelope() {
291278
let tmp = tempfile::tempdir().expect("tempdir");
292279
// Note: NO .socket/ directory at all — completely fresh tree.
293-
let (code, stdout) = run_apply(tmp.path(), &[]);
280+
let (code, stdout, _) = run_apply(tmp.path(), &["--json"], &[]);
294281
assert_eq!(code, 0, "no-manifest is not an error; stdout=\n{stdout}");
295282
let v: serde_json::Value = serde_json::from_str(&stdout).expect("envelope must be valid JSON");
296283
assert_eq!(v["command"], "apply");
@@ -380,7 +367,7 @@ fn apply_with_unreadable_socket_dir_fails_closed() {
380367
return;
381368
}
382369

383-
let (code, stdout) = run_apply(tmp.path(), &[]);
370+
let (code, stdout, _) = run_apply(tmp.path(), &["--json"], &[]);
384371
restore();
385372

386373
assert_ne!(
@@ -451,7 +438,7 @@ fn unmatched_purl_exit_semantics_are_pinned() {
451438
// with the miss visible only as a skipped event.
452439
let tmp = tempfile::tempdir().expect("tempdir");
453440
write_partial_match_project(tmp.path());
454-
let (code, stdout) = run_apply(tmp.path(), &["--offline", "--silent"]);
441+
let (code, stdout, _) = run_apply(tmp.path(), &["--json", "--offline", "--silent"], &[]);
455442
let v: serde_json::Value =
456443
serde_json::from_str(&stdout).expect("apply --json must emit valid JSON");
457444
assert_eq!(code, 0, "mixed manifest pins exit 0 today; envelope:\n{v}");
@@ -482,7 +469,7 @@ fn unmatched_purl_exit_semantics_are_pinned() {
482469
let tmp2 = tempfile::tempdir().expect("tempdir");
483470
write_partial_match_project(tmp2.path());
484471
std::fs::remove_dir_all(tmp2.path().join("node_modules")).expect("uninstall scopedpkg");
485-
let (code2, stdout2) = run_apply(tmp2.path(), &["--offline", "--silent"]);
472+
let (code2, stdout2, _) = run_apply(tmp2.path(), &["--json", "--offline", "--silent"], &[]);
486473
let v2: serde_json::Value =
487474
serde_json::from_str(&stdout2).expect("apply --json must emit valid JSON");
488475
assert_eq!(

‎crates/socket-patch-cli/tests/apply/apply_network.rs‎

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use wiremock::matchers::{method, path};
1717
use wiremock::{Mock, MockServer, ResponseTemplate};
1818

1919
use super::apply_fixture::{
20-
hashed_file, install_npm_pkg, patch_record, stage_blob, write_manifest, write_root_package_json,
20+
install_npm_pkg, one_file, patch_record, stage_blob, write_manifest, write_root_package_json,
2121
};
2222
use crate::common::{binary, git_sha256, hermetic};
2323

@@ -51,7 +51,6 @@ fn run_apply(cwd: &Path, api_url: &str, extra: &[&str]) -> (i32, String, String)
5151
async fn apply_online_fetches_missing_blob_and_patches_file() {
5252
let before = b"before\n";
5353
let after = b"after\n";
54-
let before_hash = git_sha256(before);
5554
let after_hash = git_sha256(after);
5655

5756
let mock = MockServer::start().await;
@@ -74,7 +73,7 @@ async fn apply_online_fetches_missing_blob_and_patches_file() {
7473
let socket = tmp.path().join(".socket");
7574
write_manifest(
7675
tmp.path(),
77-
json!({ purl: patch_record(uuid, hashed_file("package/index.js", &before_hash, &after_hash)) }),
76+
json!({ purl: patch_record(uuid, one_file("package/index.js", before, after)) }),
7877
);
7978

8079
let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &[]);
@@ -167,8 +166,6 @@ async fn apply_online_fetches_missing_blob_and_patches_file() {
167166
async fn apply_with_ecosystem_filter_excluding_npm_skips_all_npm_patches() {
168167
let before = b"before\n";
169168
let after = b"after\n";
170-
let before_hash = git_sha256(before);
171-
let after_hash = git_sha256(after);
172169

173170
let mock = MockServer::start().await;
174171
let purl = "pkg:npm/skipped@1.0.0";
@@ -179,7 +176,7 @@ async fn apply_with_ecosystem_filter_excluding_npm_skips_all_npm_patches() {
179176
install_npm_pkg(tmp.path(), "skipped", "1.0.0", Some(before));
180177
write_manifest(
181178
tmp.path(),
182-
json!({ purl: patch_record(uuid, hashed_file("package/index.js", &before_hash, &after_hash)) }),
179+
json!({ purl: patch_record(uuid, one_file("package/index.js", before, after)) }),
183180
);
184181

185182
let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--ecosystems", "pypi"]);
@@ -250,15 +247,14 @@ async fn apply_with_ecosystem_filter_excluding_npm_skips_all_npm_patches() {
250247
async fn apply_dry_run_emits_verified_event_without_writing() {
251248
let before = b"before\n";
252249
let after = b"after\n";
253-
let before_hash = git_sha256(before);
254250
let after_hash = git_sha256(after);
255251

256252
let tmp = tempfile::tempdir().expect("tempdir");
257253
write_root_package_json(tmp.path());
258254
install_npm_pkg(tmp.path(), "dryrun-target", "1.0.0", Some(before));
259255
write_manifest(
260256
tmp.path(),
261-
json!({ "pkg:npm/dryrun-target@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", hashed_file("package/index.js", &before_hash, &after_hash)) }),
257+
json!({ "pkg:npm/dryrun-target@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", one_file("package/index.js", before, after)) }),
262258
);
263259
// Pre-stage the after blob so we don't need to mock the network
264260
// path; we just want to verify dry-run reports the action correctly.
@@ -318,14 +314,13 @@ async fn apply_with_force_overrides_hash_mismatch() {
318314
let after_hash = git_sha256(after);
319315
let expected_before = b"expected-before\n";
320316
let actual_before = b"DIFFERENT-CONTENT\n"; // wrong before content
321-
let expected_before_hash = git_sha256(expected_before);
322317

323318
let tmp = tempfile::tempdir().expect("tempdir");
324319
write_root_package_json(tmp.path());
325320
install_npm_pkg(tmp.path(), "force-target", "1.0.0", Some(actual_before));
326321
write_manifest(
327322
tmp.path(),
328-
json!({ "pkg:npm/force-target@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", hashed_file("package/index.js", &expected_before_hash, &after_hash)) }),
323+
json!({ "pkg:npm/force-target@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", one_file("package/index.js", expected_before, after)) }),
329324
);
330325
// Pre-stage the after blob so we don't need the network.
331326
stage_blob(tmp.path(), &after_hash, after);
@@ -372,15 +367,14 @@ async fn apply_hash_mismatch_opt_in_warns_and_applies_strict_fails() {
372367
let after_hash = git_sha256(after);
373368
let expected_before = b"expected-before\n";
374369
let actual_before = b"DIFFERENT-CONTENT\n";
375-
let expected_before_hash = git_sha256(expected_before);
376370

377371
let fixture = || {
378372
let tmp = tempfile::tempdir().expect("tempdir");
379373
write_root_package_json(tmp.path());
380374
install_npm_pkg(tmp.path(), "mismatch", "1.0.0", Some(actual_before));
381375
write_manifest(
382376
tmp.path(),
383-
json!({ "pkg:npm/mismatch@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", hashed_file("package/index.js", &expected_before_hash, &after_hash)) }),
377+
json!({ "pkg:npm/mismatch@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", one_file("package/index.js", expected_before, after)) }),
384378
);
385379
stage_blob(tmp.path(), &after_hash, after);
386380
tmp
@@ -467,7 +461,6 @@ async fn apply_hash_mismatch_opt_in_warns_and_applies_strict_fails() {
467461
async fn apply_pypi_package_uses_python_crawler() {
468462
let before = b"def hello():\n return 'before'\n";
469463
let after = b"def hello():\n return 'after'\n";
470-
let before_hash = git_sha256(before);
471464
let after_hash = git_sha256(after);
472465

473466
let tmp = tempfile::tempdir().expect("tempdir");
@@ -503,7 +496,7 @@ async fn apply_pypi_package_uses_python_crawler() {
503496

504497
write_manifest(
505498
tmp.path(),
506-
json!({ "pkg:pypi/pypi_target@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", hashed_file("package/index.js", &before_hash, &after_hash)) }),
499+
json!({ "pkg:pypi/pypi_target@1.0.0": patch_record("11111111-1111-4111-8111-111111111111", one_file("package/index.js", before, after)) }),
507500
);
508501
stage_blob(tmp.path(), &after_hash, after);
509502

@@ -552,7 +545,6 @@ async fn apply_pypi_package_uses_python_crawler() {
552545
async fn apply_uses_locally_cached_blob_without_fetching() {
553546
let before = b"before\n";
554547
let after = b"after\n";
555-
let before_hash = git_sha256(before);
556548
let after_hash = git_sha256(after);
557549

558550
let tmp = tempfile::tempdir().expect("tempdir");
@@ -561,7 +553,7 @@ async fn apply_uses_locally_cached_blob_without_fetching() {
561553
let socket = tmp.path().join(".socket");
562554
write_manifest(
563555
tmp.path(),
564-
json!({ "pkg:npm/cached@1.0.0": patch_record("22222222-2222-4222-8222-222222222222", hashed_file("package/index.js", &before_hash, &after_hash)) }),
556+
json!({ "pkg:npm/cached@1.0.0": patch_record("22222222-2222-4222-8222-222222222222", one_file("package/index.js", before, after)) }),
565557
);
566558
stage_blob(tmp.path(), &after_hash, after);
567559

@@ -613,7 +605,6 @@ async fn apply_uses_locally_cached_blob_without_fetching() {
613605
async fn apply_mismatch_redownloads_full_blob_and_applies() {
614606
let after = b"after\n";
615607
let after_hash = git_sha256(after);
616-
let expected_before_hash = git_sha256(b"expected-before\n");
617608

618609
let mock = MockServer::start().await;
619610
Mock::given(method("GET"))
@@ -636,7 +627,10 @@ async fn apply_mismatch_redownloads_full_blob_and_applies() {
636627
let socket = tmp.path().join(".socket");
637628
write_manifest(
638629
tmp.path(),
639-
json!({ "pkg:npm/mismatch@1.0.0": patch_record(uuid, hashed_file("package/index.js", &expected_before_hash, &after_hash)) }),
630+
json!({ "pkg:npm/mismatch@1.0.0": patch_record(
631+
uuid,
632+
one_file("package/index.js", b"expected-before\n", after),
633+
)}),
640634
);
641635
// A leftover LOCAL package archive exists, but nothing reads it: only
642636
// the blob can produce the patched bytes, and no blob is cached.
@@ -705,7 +699,6 @@ async fn apply_mismatch_redownloads_full_blob_and_applies() {
705699
async fn offline_apply_with_token_makes_zero_network_requests() {
706700
let before = b"offline airgap before\n";
707701
let after = b"offline airgap after\n";
708-
let before_hash = git_sha256(before);
709702
let after_hash = git_sha256(after);
710703

711704
// No mocks mounted: every request is a violation, and all are recorded.
@@ -716,7 +709,7 @@ async fn offline_apply_with_token_makes_zero_network_requests() {
716709
install_npm_pkg(tmp.path(), "airgap-test", "1.0.0", Some(before));
717710
write_manifest(
718711
tmp.path(),
719-
json!({ "pkg:npm/airgap-test@1.0.0": patch_record("33333333-3333-4333-8333-333333333333", hashed_file("package/index.js", &before_hash, &after_hash)) }),
712+
json!({ "pkg:npm/airgap-test@1.0.0": patch_record("33333333-3333-4333-8333-333333333333", one_file("package/index.js", before, after)) }),
720713
);
721714
stage_blob(tmp.path(), &after_hash, after);
722715

@@ -798,7 +791,6 @@ fn write_package_archive(packages: &Path, uuid: &str, entries: &[(&str, &[u8])])
798791
async fn apply_online_ignores_legacy_package_archive_when_downloads_fail() {
799792
let before = b"pkgcache before\n";
800793
let after = b"pkgcache after\n";
801-
let before_hash = git_sha256(before);
802794
let after_hash = git_sha256(after);
803795
let uuid = "44444444-4444-4444-8444-444444444444";
804796

@@ -812,7 +804,7 @@ async fn apply_online_ignores_legacy_package_archive_when_downloads_fail() {
812804
let socket = tmp.path().join(".socket");
813805
write_manifest(
814806
tmp.path(),
815-
json!({ "pkg:npm/pkgcache@1.0.0": patch_record(uuid, hashed_file("package/index.js", &before_hash, &after_hash)) }),
807+
json!({ "pkg:npm/pkgcache@1.0.0": patch_record(uuid, one_file("package/index.js", before, after)) }),
816808
);
817809
write_package_archive(
818810
&socket.join("packages"),
@@ -857,7 +849,6 @@ async fn apply_online_ignores_legacy_package_archive_when_downloads_fail() {
857849
async fn mismatch_overwrites_every_copy_of_a_duplicated_package() {
858850
let before = b"before\n";
859851
let after = b"after\n";
860-
let before_hash = git_sha256(before);
861852
let after_hash = git_sha256(after);
862853

863854
let mock = MockServer::start().await;
@@ -889,7 +880,7 @@ async fn mismatch_overwrites_every_copy_of_a_duplicated_package() {
889880
let socket = tmp.path().join(".socket");
890881
write_manifest(
891882
tmp.path(),
892-
json!({ purl: patch_record(uuid, hashed_file("package/index.js", &before_hash, &after_hash)) }),
883+
json!({ purl: patch_record(uuid, one_file("package/index.js", before, after)) }),
893884
);
894885
let (code, stdout, stderr) = run_apply(tmp.path(), &mock.uri(), &["--allow-patch-conflicts"]);
895886
let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON");

‎crates/socket-patch-cli/tests/apply/apply_render.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
//! `Would remove` announcement, "No patches to apply.", the
88
//! no-matching-package error block, the dry-run summary counts,
99
//! `--verbose` per-file labels, the pnpm/bun/vlt layout notes, the
10-
//! yarn-PnP refusal, the gem fallback-home skip warning, the gem
10+
//! yarn-PnP refusal, the gem
1111
//! variant mismatch warning and no-matching-variant error, the
1212
//! `--dry-run --vex` skip line, and the staging layer's offline
1313
//! no-local-source report and download result line.

‎crates/socket-patch-cli/tests/apply/bun_global_store.rs‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use std::path::{Path, PathBuf};
1212

1313
use serde_json::Value;
1414

15-
use super::apply_fixture::{link_dir, one_file, patch_record, stage_blob, write_manifest};
15+
use super::apply_fixture::{index_js_patches, link_dir, stage_blob, write_manifest};
1616
use crate::common::envelope::event_for;
1717
use crate::common::{git_sha256, parse_json_envelope, run_with_env};
1818

@@ -71,16 +71,7 @@ fn stage(tmp: &Path, bytes: &[u8]) -> (PathBuf, PathBuf, PathBuf) {
7171
}
7272

7373
fn write_patches(root: &Path, purls: &[&str]) {
74-
let patches = purls
75-
.iter()
76-
.enumerate()
77-
.map(|(i, purl)| {
78-
let uuid = format!("63563563-0000-4000-8000-00000000000{i}");
79-
let record = patch_record(&uuid, one_file("package/index.js", BEFORE, AFTER));
80-
(purl.to_string(), record)
81-
})
82-
.collect();
83-
write_manifest(root, Value::Object(patches));
74+
write_manifest(root, index_js_patches(purls, "63563563", BEFORE, AFTER));
8475
for bytes in [BEFORE, AFTER] {
8576
stage_blob(root, &git_sha256(bytes), bytes);
8677
}

0 commit comments

Comments
 (0)