crates/config/src/workspace_yaml/tests.rs (1)

452-452: ⚡ Quick win

Add failure context to the boolean assertion.

The assert! here will be harder to diagnose on failure without printing actual hosts. Add a message (or nearby eprintln!) with config.git_shallow_hosts.

Based on learnings: In Rust test code, assertions like assert! should log relevant actual/expected context so failures are diagnosable without reruns.

🤖 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/config/src/workspace_yaml/tests.rs` at line 452, The boolean assertion
on config.git_shallow_hosts lacks failure context; update the test to include
the actual hosts when the assertion fails by replacing the bare assert! with one
that supplies a diagnostic message containing config.git_shallow_hosts (e.g.,
assert!(config.git_shallow_hosts.iter().any(|h| h == "github.com"),
"git_shallow_hosts = {:?}", config.git_shallow_hosts)) or alternatively print
config.git_shallow_hosts with eprintln! immediately before the assertion so
failures show the real value.

crates/git-fetcher/src/prepare_package/tests.rs (1)

37-43: ⚡ Quick win

Improve failure diagnostics for boolean assertions in tests.

Several assert!/matches! checks here have no extra context, which makes failures harder to triage. Add assertion messages (or eprintln! context) for the key actual values.

Based on learnings: In Rust test code, assertions like assert!/assert_ne! should include diagnostic logging/context unless assert_eq! already provides the necessary diff.

Also applies to: 45-52, 55-64, 67-75, 77-84, 87-95, 98-113, 137-143, 146-150

🤖 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/git-fetcher/src/prepare_package/tests.rs` around lines 37 - 43, The
boolean assertions in tests.rs (e.g., the test function
package_should_be_built_false_for_empty_scripts and other tests listed) lack
diagnostic context; update each assert!/matches! call to include a failure
message or emit contextual eprintln! output that shows the actual value(s) under
test (for example the returned bool from package_should_be_built, the manifest
JSON, or computed script lists). Locate usages of package_should_be_built and
any other boolean/matches assertions in this file (tests around lines
referenced: 45-52, 55-64, 67-75, 77-84, 87-95, 98-113, 137-143, 146-150) and
change asserts to include formatted messages like "expected X but got {:?}", or
print the relevant variables before asserting so test failures report the
concrete values.

crates/git-fetcher/src/packlist/tests.rs (2)

115-117: ⚡ Quick win

Add diagnostic logging before assertions.

These assert! calls should log the actual out value before the assertion so that test failures can be diagnosed without re-running.

Based on learnings, when using assert!, assert_ne!, etc. (not assert_eq!), ensure the test logs relevant actual/expected values before the assertion.

🔍 Proposed fix

+    eprintln!("packlist output: {out:?}");
     assert!(out.contains(&"lib/index.js".to_string()));
     assert!(out.contains(&"bin/cli".to_string()));
     assert!(out.contains(&"dist/index.js".to_string()));

🤖 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/git-fetcher/src/packlist/tests.rs` around lines 115 - 117, Before the
three assertions that check out.contains(...) in the test in
crates/git-fetcher/src/packlist/tests.rs, add diagnostic logging that prints the
value of the variable out (e.g., via eprintln! or debug!) so failing assertions
show the actual contents; update the test around the assertions that reference
"lib/index.js", "bin/cli", and "dist/index.js" to emit out before each assert
(or once before the group) so test failures can be diagnosed without re-running.

---

136-137: ⚡ Quick win

Add diagnostic logging before assertions.

These assert! calls should log the actual out value so test failures can be diagnosed without re-running.

Based on learnings, assertions that don't automatically show the differing values (like assert! and assert_ne!) should be preceded by logging the relevant context.

🔍 Proposed fix

+    eprintln!("packlist output: {out:?}");
     assert!(out.contains(&"lib/index.js".to_string()));
     assert!(!out.contains(&"lib/sub/inner.js".to_string()));

🤖 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/git-fetcher/src/packlist/tests.rs` around lines 136 - 137, Add
diagnostic logging that prints the `out` value immediately before the two
assertions so test failures show the actual content; locate the test in
crates/git-fetcher/src/packlist/tests.rs where `out` is asserted against
"lib/index.js" and "lib/sub/inner.js" and insert a diagnostic print (e.g.,
eprintln! or dbg!) of `out` right before those assert! calls to surface the
failing value during test runs.

crates/git-fetcher/src/fetcher/tests.rs (1)

102-106: ⚡ Quick win

Add diagnostic logging of the full result before assertions.

The test makes multiple assertions on received but doesn't log the full structure first. If one of these assertions fails, it would be helpful to see the entire returned value.

Based on learnings, tests should log diagnostic values around assertions (except assert_eq! which already shows diffs) so failures can be diagnosed without rerunning.

🔍 Proposed fix

+    eprintln!("GitFetcher returned: {received:#?}");
     assert!(!received.built, "package without scripts should not be 'built'");
     assert!(received.cas_paths.contains_key("package.json"));
     assert!(received.cas_paths.contains_key("index.js"));

🤖 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/git-fetcher/src/fetcher/tests.rs` around lines 102 - 106, Add
diagnostic logging of the full test result `received` before the assertions so
failures show the returned structure; locate the test in
crates/git-fetcher/src/fetcher/tests.rs where `received` is asserted and insert
a debug/log statement (e.g., using dbg! or formatted debug output) immediately
before the first assertion; ensure the log prints the entire `received`
(borrowed or cloned as needed) but keep the rest of the assertions
(`received.built`, `received.cas_paths`, `cas_path.exists()`) unchanged.

crates/git-fetcher/src/error.rs (1)

7-7: ⚡ Quick win

Use pnpm/blob/main links instead of pinned SHA links in porting docs.

Please update these upstream references to https://github.com/pnpm/pnpm/blob/main/... so they always track pnpm’s current source of truth.

Suggested doc-link update

-/// [`exec/prepare-package`](https://github.com/pnpm/pnpm/blob/94240bc046/exec/prepare-package/src/index.ts).
+/// [`exec/prepare-package`](https://github.com/pnpm/pnpm/blob/main/exec/prepare-package/src/index.ts).

-    /// Mirrors [`exec/prepare-package/src/index.ts:37-46`](https://github.com/pnpm/pnpm/blob/94240bc046/exec/prepare-package/src/index.ts#L37-L46).
+    /// Mirrors [`exec/prepare-package/src/index.ts:37-46`](https://github.com/pnpm/pnpm/blob/main/exec/prepare-package/src/index.ts#L37-L46).

-    /// [`exec/prepare-package/src/index.ts:71-77`](https://github.com/pnpm/pnpm/blob/94240bc046/exec/prepare-package/src/index.ts#L71-L77).
+    /// [`exec/prepare-package/src/index.ts:71-77`](https://github.com/pnpm/pnpm/blob/main/exec/prepare-package/src/index.ts#L71-L77).

-    /// at [`exec/prepare-package/src/index.ts:92-103`](https://github.com/pnpm/pnpm/blob/94240bc046/exec/prepare-package/src/index.ts#L92-L103).
+    /// at [`exec/prepare-package/src/index.ts:92-103`](https://github.com/pnpm/pnpm/blob/main/exec/prepare-package/src/index.ts#L92-L103).

-    /// [`fetching/git-fetcher/src/index.ts:39-41`](https://github.com/pnpm/pnpm/blob/94240bc046/fetching/git-fetcher/src/index.ts#L39-L41).
+    /// [`fetching/git-fetcher/src/index.ts:39-41`](https://github.com/pnpm/pnpm/blob/main/fetching/git-fetcher/src/index.ts#L39-L41).

As per coding guidelines: “When reading on GitHub, open files from https://github.com/pnpm/pnpm/blob/main/... rather than from a permalinked SHA.”

Also applies to: 15-15, 29-29, 39-39, 87-87

🤖 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/git-fetcher/src/error.rs` at line 7, Replace the pinned SHA GitHub
links in the crate's doc comments with canonical `blob/main` references so they
track pnpm’s current source; specifically update the doc-comment that references
`exec/prepare-package` (and the other occurrences of pinned pnpm links in this
file) to use `https://github.com/pnpm/pnpm/blob/main/...` instead of the
permalinked SHA. Locate the doc comments in crates/git-fetcher/src/error.rs that
contain the pnpm links (e.g., the comment mentioning `exec/prepare-package`) and
swap the `/blob/<SHA>/` portion for `/blob/main/` for all occurrences in this
file.