crates/reporter/src/tests.rs (1)

617-637: 💤 Low value

Consider adding diagnostic logging closer to the assertion.

The dbg! call is several lines before the assertion that checks details absence. Adding a focused log immediately before line 636 would improve test debuggability per CODE_STYLE_GUIDE.

Suggested addition

     let json: Value = envelope
         .pipe_ref(serde_json::to_string)
         .expect("serialize envelope")
         .pipe_as_ref(serde_json::from_str)
         .expect("parse JSON");
+    eprintln!("json.get(\"details\") = {:?}", json.get("details"));
     assert!(json.get("details").is_none(), "details must be omitted when absent, got {json:?}",);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/reporter/src/tests.rs` around lines 617 - 637, In the test
skipped_optional_omits_absent_details, add a focused diagnostic print
immediately before the assert that checks json.get("details").is_none() so
failures show the exact JSON under test; e.g., log the `json` Value or
`json.get("details")` right before the assertion (where `envelope` is serialized
into `json`) to improve debuggability of the failing assertion in this test.

crates/lockfile/src/snapshot_entry/tests.rs (2)

27-38: ⚡ Quick win

Add diagnostic logging before assertions.

Same issue as the previous test—add explicit logging before assert! calls per CODE_STYLE_GUIDE. Based on learnings, add logging (e.g., eprintln!/eprintln!(...)) so that useful diagnostic values are printed when a test fails, unless the assertion is assert_eq! (where the differing values are already included).

Suggested logging additions

 #[test]
 fn optional_defaults_false_and_omits_when_false() {
     let yaml = text_block! {
         "dependencies:"
         "  bar: 1.0.0"
     };
     let entry: SnapshotEntry = serde_saphyr::from_str(yaml).expect("parse");
+    eprintln!("entry.optional = {}", entry.optional);
     assert!(!entry.optional, "default must be false when absent");
 
     let out = serialize_yaml::to_string(&entry).expect("serialize");
+    eprintln!("serialized output:\n{}", out);
     assert!(!out.contains("optional"), "false must not be serialized:\n{out}");
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/lockfile/src/snapshot_entry/tests.rs` around lines 27 - 38, Add
diagnostic logging in the test optional_defaults_false_and_omits_when_false
before each assert: print the parsed SnapshotEntry optional flag (e.g.,
eprintln!("entry.optional = {:?}", entry.optional)) before the
assert!(!entry.optional...) and print the serialized YAML string (e.g.,
eprintln!("serialized out = {}", out)) before the
assert!(!out.contains("optional")...) so failing assertions emit useful
diagnostics; keep logs simple and use eprintln! to match the other tests' style.

---

10-22: ⚡ Quick win

Add diagnostic logging before assertions.

Per the CODE_STYLE_GUIDE learning, tests should include eprintln! or dbg! to log actual values before assert! calls (not just in the assertion message) so failures can be diagnosed without rerunning. Based on learnings, add logging (e.g., eprintln!/eprintln!(...)) so that useful diagnostic values are printed when a test fails, unless the assertion is assert_eq! (where the differing values are already included).

Suggested logging additions

 #[test]
 fn optional_true_round_trips() {
     let yaml = text_block! {
         "dependencies:"
         "  foo: 1.2.3"
         "optional: true"
     };
     let entry: SnapshotEntry = serde_saphyr::from_str(yaml).expect("parse");
+    eprintln!("entry.optional = {}", entry.optional);
     assert!(entry.optional, "deserialize must capture optional: true");
 
     let out = serialize_yaml::to_string(&entry).expect("serialize");
+    eprintln!("serialized output:\n{}", out);
     assert!(out.contains("optional: true"), "serialize must round-trip optional: true:\n{out}");
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/lockfile/src/snapshot_entry/tests.rs` around lines 10 - 22, The test
optional_true_round_trips lacks diagnostic logging before assertions; add
eprintln! (or dbg!) calls to print the deserialized SnapshotEntry state and the
serialized output so failures are easier to diagnose: log entry.optional (or the
entire entry) right after let entry: SnapshotEntry = ... and log the serialized
out right after serialize_yaml::to_string(&entry).expect(...), keeping the
existing asserts unchanged.

crates/package-manager/src/build_modules/tests.rs (1)

320-335: ⚡ Quick win

Strengthen the optional-failure assertions to catch silent regressions.

This test currently logs ignored but does not assert it; it also accepts empty details as long as it is Some. Tightening both checks will make the behavior contract explicit.

Proposed test hardening

-    dbg!(&ignored);
+    assert!(
+        ignored.is_empty(),
+        "optional build failure should not add ignored scripts: {ignored:?}",
+    );

@@
-    assert!(skipped_event.details.is_some(), "details must carry the error toString");
+    let details = skipped_event.details.as_deref().unwrap_or("");
+    assert!(
+        !details.trim().is_empty(),
+        "details must carry non-empty lifecycle error context: {skipped_event:?}",
+    );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/package-manager/src/build_modules/tests.rs` around lines 320 - 335,
The test currently prints `ignored` and only checks `details.is_some()`, which
allows silent regressions; update the assertions to (1) assert that the
`ignored` collection (variable `ignored`) contains the expected entry(s) for the
failing-postinstall package (i.e., not empty and/or contains the package name),
and (2) assert that `skipped_event.details` is a non-empty string (not just
Some) so it actually carries the error text; locate the test where `ignored` is
created and the `skipped_event` is extracted from `EVENTS` (matching
`LogEvent::SkippedOptionalDependency`) and replace the weak checks with concrete
assertions that `ignored` includes the failing package and
`skipped_event.details.as_ref().map(|s| !s.trim().is_empty()).unwrap_or(false)`
is true while keeping the existing checks for
`SkippedOptionalReason::BuildFailure` and package name/version.

• tasks/registry-mock/pnpm-lock.yaml: Language not supported

crates/package-manager/src/install/tests.rs (1)

708-724: ⚡ Quick win

Add diagnostic logs around assert! checks in the new test.

Lines 708-724 use assert! checks without printing the evaluated runtime state, which makes CI failures harder to triage.

Suggested patch

-    assert!(
-        is_symlink_or_junction(
-            &project_root.join("node_modules/@pnpm.e2e/has-failing-postinstall-dep"),
-        )
-        .unwrap(),
-        "wrapper symlink missing",
-    );
+    let wrapper_link = project_root.join("node_modules/@pnpm.e2e/has-failing-postinstall-dep");
+    eprintln!(
+        "wrapper_link={wrapper_link:?} symlink_or_junction={:?}",
+        is_symlink_or_junction(&wrapper_link)
+    );
+    assert!(
+        is_symlink_or_junction(&wrapper_link).unwrap(),
+        "wrapper symlink missing",
+    );
-    assert!(
-        project_root
-            .join("node_modules/.pacquet/@pnpm.e2e+has-failing-postinstall-dep@1.0.0")
-            .is_dir(),
-        "wrapper virtual-store dir missing",
-    );
+    let wrapper_store_dir =
+        project_root.join("node_modules/.pacquet/@pnpm.e2e+has-failing-postinstall-dep@1.0.0");
+    eprintln!(
+        "wrapper_store_dir={wrapper_store_dir:?} is_dir={}",
+        wrapper_store_dir.is_dir()
+    );
+    assert!(wrapper_store_dir.is_dir(), "wrapper virtual-store dir missing");
-    assert!(
-        project_root.join("node_modules/.pacquet/@pnpm.e2e+failing-postinstall@1.0.0").is_dir(),
-        "transitive `failing-postinstall` must be extracted to the virtual store",
-    );
+    let transitive_store_dir =
+        project_root.join("node_modules/.pacquet/@pnpm.e2e+failing-postinstall@1.0.0");
+    eprintln!(
+        "transitive_store_dir={transitive_store_dir:?} is_dir={}",
+        transitive_store_dir.is_dir()
+    );
+    assert!(
+        transitive_store_dir.is_dir(),
+        "transitive `failing-postinstall` must be extracted to the virtual store",
+    );

Based on learnings: “In Rust test code, follow the repo’s CODE_STYLE_GUIDE test-logging rule: add logging (e.g., eprintln!) for assert!/assert_ne!-style assertions so failures are diagnosable.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/package-manager/src/install/tests.rs` around lines 708 - 724, The
three assert! checks around is_symlink_or_junction(...) and the
project_root.join(...) path existence checks lack diagnostic output; before each
assert!, print the evaluated runtime state (e.g., the boolean result of
is_symlink_or_junction, the full path strings, and whether .is_dir() returned
true) using eprintln! so CI failures show actual values; specifically, for the
is_symlink_or_junction call store its Result/boolean and eprintln! the path and
result before asserting, and for the two .is_dir() checks eprintln! the joined
path and the .is_dir() boolean before the assert!.