test(outdated): add regression test for minimumReleaseAge#11699
Conversation
📝 WalkthroughWalkthroughThis PR adds a Jest test module validating that ChangesOutdated Maturity Age Test Coverage
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@zkochan treat this more as a solid repro with the test if you want. Feel welcome to close the PR and reimplement it rather than talk me through making it perfect (assuming the issue itself is valid). |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deps/inspection/commands/test/outdated/minimumReleaseAge.test.ts (1)
74-86: ⚡ Quick winConsider asserting
exitCodefor more complete validation.Since
allImmatureMinimumReleaseAgemakes every version immature, the expected behavior is thatoutdatedfinds no upgradeable packages at all. The test currently only verifies that2.1.0doesn't appear, but adding an assertion thatexitCodeis0would confirm the complete intended behavior: no outdated packages reported when all versions are within the maturity window.🧪 Suggested enhancement
test('pnpm outdated honors minimumReleaseAge: immature newer versions are not offered', async () => { loadHasOutdatedDeps() - const { output } = await outdated.handler({ + const { output, exitCode } = await outdated.handler({ ...OUTDATED_OPTIONS, dir: process.cwd(), minimumReleaseAge: allImmatureMinimumReleaseAge, }) // 2.1.0 is far newer than the (epoch) cutoff, so a correct age filter must // not present it as an available upgrade. expect(stripAnsi(output)).not.toContain('2.1.0') + // With all versions immature, no packages should be reported as outdated. + expect(exitCode).toBe(0) })🤖 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 `@deps/inspection/commands/test/outdated/minimumReleaseAge.test.ts` around lines 74 - 86, The test "pnpm outdated honors minimumReleaseAge: immature newer versions are not offered" currently only asserts that '2.1.0' is not in output; update the assertion to also verify the handler returned a successful exit by asserting the returned result's exitCode is 0 (the call to outdated.handler that spreads OUTDATED_OPTIONS with dir/process.cwd() and minimumReleaseAge: allImmatureMinimumReleaseAge returns { output, exitCode }). Add an assertion like expect(exitCode).toBe(0) after the call to ensure no upgradeable packages were reported.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@deps/inspection/commands/test/outdated/minimumReleaseAge.test.ts`:
- Around line 74-86: The test "pnpm outdated honors minimumReleaseAge: immature
newer versions are not offered" currently only asserts that '2.1.0' is not in
output; update the assertion to also verify the handler returned a successful
exit by asserting the returned result's exitCode is 0 (the call to
outdated.handler that spreads OUTDATED_OPTIONS with dir/process.cwd() and
minimumReleaseAge: allImmatureMinimumReleaseAge returns { output, exitCode }).
Add an assertion like expect(exitCode).toBe(0) after the call to ensure no
upgradeable packages were reported.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c3244fd2-27f6-47a0-ac1b-64659cbb0f2d
📒 Files selected for processing (3)
.changeset/outdated-minimum-release-age-strict.mddeps/inspection/commands/test/outdated/minimumReleaseAge.test.tsdeps/inspection/outdated/src/createManifestGetter.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use Standard Style with modifications: trailing commas are used, functions are preferred over classes, functions are declared after they are used (hoisting is relied upon).
Functions should have no more than two or three arguments. If a function needs more parameters, use a single options object instead.
Maintain import order: (1) Standard libraries, (2) External dependencies (sorted alphabetically), (3) Relative imports.
Files:
deps/inspection/outdated/src/createManifestGetter.tsdeps/inspection/commands/test/outdated/minimumReleaseAge.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use
instanceof Errorfor error type checking in Jest tests. Useutil.types.isNativeError()instead, as Jest runs tests in a VM context whereinstanceofchecks can fail across realms.
Files:
deps/inspection/commands/test/outdated/minimumReleaseAge.test.ts
🧠 Learnings (1)
📚 Learning: 2026-05-14T09:04:00.133Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11622
File: resolving/npm-resolver/test/publishedBy.test.ts:350-354
Timestamp: 2026-05-14T09:04:00.133Z
Learning: In the pnpm/pnpm repository, ESLint is the authoritative style linter. Do not raise review findings for missing trailing commas in multiline function calls (e.g., `fs.writeFileSync(...)`) when this repo’s ESLint configuration does not report them and lint passes. Prefer deferring to the ESLint results for this specific trailing-comma rule rather than enforcing it manually in code review.
Applied to files:
deps/inspection/outdated/src/createManifestGetter.tsdeps/inspection/commands/test/outdated/minimumReleaseAge.test.ts
🔇 Additional comments (4)
deps/inspection/outdated/src/createManifestGetter.ts (1)
32-39: LGTM!deps/inspection/commands/test/outdated/minimumReleaseAge.test.ts (2)
1-32: LGTM!
34-45: LGTM!.changeset/outdated-minimum-release-age-strict.md (1)
1-8: LGTM!
Adds a regression test verifying that `pnpm outdated` honors `minimumReleaseAge` by not surfacing immature newer versions as available upgrades. The underlying fix already shipped as part of pnpm#11705 (which removed `strictPublishedByCheck` entirely and routed maturity decisions through `policyViolation`); this lands the dedicated test from the original repro PR (pnpm#11699).
Adds `expect(exitCode).toBe(0)` and trims the now-stale pre-fix narrative above the test. Addresses the CodeRabbit review nit on pnpm#11699.
3851b4f to
1a6f015
Compare
|
Congrats on merging your first pull request! 🎉🎉🎉 |
Adds a regression test for #11698. The underlying fix already shipped as part of #11705 (which removed
strictPublishedByCheckentirely and routed maturity decisions throughpolicyViolation), so this PR now lands only the dedicated test that locks in the behavior.What the test covers
deps/inspection/commands/test/outdated/minimumReleaseAge.test.ts:pnpm outdatedreportsis-negative@2.1.0as an available upgrade (sanity check that the fixture actually has outdated deps).minimumReleaseAgeset to a cutoff so far in the past that every published version is immature,pnpm outdatedreports nothing:exitCode === 0and2.1.0does not appear. Before feat: tighten minimumReleaseAge — auto-exclude, lockfile verification, and interactive prompt #11705 this test went red because the non-strict resolver fallback re-picked the immaturelatestignoringpublishedBy.The
allImmatureMinimumReleaseAge = Date.now() / (60 * 1000)trick (cutoff = epoch in minutes) is date-independent and matches the technique already used in the install-sideminimumReleaseAgesuite.Why a test-only PR
The original PR proposed flipping
strictPublishedByCheckincreateManifestGetter, but #11705 deleted that option entirely and replaced it with an always-defer model (policyViolationflows throughResolveResult→getManifestreturnsnullonMINIMUM_RELEASE_AGE_VIOLATION). The test was the durable contribution; preserving it as a regression gate is worth keeping.Written by an agent (Claude Code, claude-opus-4-7).