Skip to content

test: port the four unblocked pnpm hooks.ts cases#455

Merged
jdx merged 3 commits intomainfrom
claude/vigilant-maxwell-b88826
May 1, 2026
Merged

test: port the four unblocked pnpm hooks.ts cases#455
jdx merged 3 commits intomainfrom
claude/vigilant-maxwell-b88826

Conversation

@jdx
Copy link
Copy Markdown
Contributor

@jdx jdx commented May 1, 2026

Summary

Ports the four pnpm/test/install/hooks.ts cases that PNPM_TEST_IMPORT.md marked "Phase 0 unblocked" — the Tier 1 fixtures (@pnpm.e2e/*) and add_dist_tag helper that previously gated them have already landed, so this is just the test-port follow-up.

pnpm hooks.ts aube test
:18 sync readPackage hook pnpmfile: sync readPackage hook pins a transitive dep
:217 workspace-root pnpmfile pnpmfile: workspace-root pnpmfile applies during install and sub-project add
:404 global + local ctx.log composition pnpmfile: global + local readPackage ctx.log emits one pnpm:hook record per pnpmfile
:661 shared workspace lockfile pnpmfile: readPackage with shared workspace lockfile rewrites importer deps (skipped)

Notes

  • :18 is the sync sibling of the existing async port — same fixture, same expectation, just exercises the non-async hook code path.
  • :217 exercises pnpmfile discovery from two angles: a workspace-root aube install and a subsequent aube add from a member. Both walk up to the workspace-root cwd, so the same root pnpmfile fires in both — the test asserts hook-injected transitives land in the shared aube-lock.yaml snapshots for both is-positive@1.0.0 and is-negative@1.0.0. The aube add runs inside a bash -c 'cd project-1 && …' subshell to keep run semantics intact while sidestepping shellcheck SC2103 on a bare cd ...
  • :404 reuses the global-then-local compose pattern that landed earlier in this file but verifies the ndjson reporter side: two pnpm:hook readPackage records, same prefix, distinct from, in global-then-local order.
  • :661 is ported as skip because it relies on readPackage firing on importer manifests (root or workspace project) — same root cause as the existing :551 skip in this file. aube only fires readPackage on resolved registry packages. Filed alongside the other documented divergence; the skip note points at the same Discussion gate.
  • While re-running the count, noticed hooks.ts:580 (readPackage during aube remove in a workspace) was unaccounted for in the previous PNPM_TEST_IMPORT.md ledger. Added it to the "Not yet ported" line so the 22-test total balances (17 ported + 3 skipped + 1 not yet ported + 1 covered elsewhere = 22).

Test plan

  • cargo fmt --check clean
  • cargo build clean
  • test/bats/bin/bats test/pnpm_install_hooks.bats — 20/20 pass (17 active + 3 documented divergence skips)
  • pre-commit shfmt + shellcheck clean

🤖 Generated with Claude Code


Note

Low Risk
Low risk: changes are limited to test suite additions and a documentation ledger update, with no production logic modifications.

Overview
Adds four new pnpmfile parity tests in test/pnpm_install_hooks.bats covering the sync readPackage path, workspace-root pnpmfile discovery during both aube install and member aube add, and ndjson logging for global+local pnpmfile composition.

Updates test/PNPM_TEST_IMPORT.md to reflect the increased hooks.ts porting coverage and to track one remaining unported workspace-remove hook case; the shared-workspace-lockfile importer-rewrite test is included but explicitly skipped as an aube divergence.

Reviewed by Cursor Bugbot for commit 5f09b0c. Bugbot is set up for automated code reviews on this repo. Configure here.

Ports the four pnpm/test/install/hooks.ts cases tracked as "Phase 0
unblocked" in test/PNPM_TEST_IMPORT.md:

- sync readPackage hook (hooks.ts:18) — sync sibling of the existing
  async port, verbatim against the mirrored @pnpm.e2e/pkg-with-1-dep
  fixture.
- workspace-root pnpmfile applies during install + sub-project add
  (hooks.ts:217) — root .pnpmfile.cjs fires both during a workspace
  `aube install` from the root and a subsequent `aube add` from a
  member, since both walk up to the workspace-root cwd. Asserts the
  hook-injected transitive lands in the shared aube-lock.yaml.
- global + local readPackage ctx.log composition (hooks.ts:404) —
  asserts two pnpm:hook ndjson records (same prefix, distinct from)
  appear in global-then-local order under --reporter=ndjson.
- shared workspace lockfile rewrites importer deps (hooks.ts:661) —
  ported as skip with a divergence note pointing to the same root
  cause as the existing 551 skip: aube only fires readPackage on
  resolved registry packages, never on importer manifests.

While updating PNPM_TEST_IMPORT.md to reflect the new state, noticed
hooks.ts:580 (readPackage during `aube remove` in a workspace) was
unaccounted for in the original ledger and added it to the "Not yet
ported" line so the 22-test total balances.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 1, 2026

Greptile Summary

Ports four pnpm/test/install/hooks.ts cases to Bats: a sync readPackage path (:18), workspace-root pnpmfile discovery across aube install + member aube add (:217), global+local ctx.log ndjson verification (:404), and an explicit skip documenting the importer-manifest divergence (:661). The ledger in PNPM_TEST_IMPORT.md is updated consistently, and one newly noticed unported case (:580) is correctly added to the tracking table.

Confidence Score: 5/5

Safe to merge — test-only changes with no production logic affected.

All four new tests are careful ports with well-scoped fixtures, thorough inline comments explaining any aube/pnpm divergence, and no changes to production code. The awk-based snapshot extraction in the workspace test is resilient to future YAML field insertions. The skipped divergence is clearly documented with the root cause and an explicit gate before un-skipping.

No files require special attention.

Important Files Changed

Filename Overview
test/pnpm_install_hooks.bats Adds four new Bats tests porting pnpm hooks.ts cases: sync readPackage, workspace-root pnpmfile discovery, global+local ctx.log ndjson verification, and a documented skip for the importer-manifest divergence.
test/PNPM_TEST_IMPORT.md Ledger updated accurately: ported count advances from 14→17, skipped divergences from 2→3, and one new unported case (hooks.ts:580) is added to the Not yet ported line.

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread test/pnpm_install_hooks.bats Outdated
… lockfile-resilient

- The compose-log test used `mapfile -t`, which is bash 4+. macOS CI
  runners still ship bash 3.2, so the test failed on
  `bats (macos-latest, …, 1, 2)` with `mapfile: command not found`.
  Replace with `sed -n 'Np'` line extraction — portable to bash 3.2.

- The workspace-root pnpmfile assertions used `grep -A2` to scope the
  match window inside each lockfile snapshot block. -A2 is too narrow
  if a snapshot block ever gains a `resolution:` (or similar) field
  before `dependencies:`, in which case the dep key would silently
  fall outside the window. Switch to a nested-awk extraction that
  walks from the snapshot key forward to the next sibling at 2-space
  indent, matching the actual block boundary regardless of internal
  field order or count.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Benchmark changes

Public ratios: warm installs vs Bun 5x -> 6x; warm installs vs pnpm 6x -> 8x.

Benchmark aube bun pnpm
Fresh install (warm cache) 288ms -> 339ms (+18%) 1504ms -> 1964ms (+31%) 1849ms -> 2749ms (+49%)
CI install (warm cache, GVS disabled) 416ms -> 1130ms (+172%) 966ms -> 2166ms (+124%) 2237ms -> 2743ms (+23%)
CI install (cold cache, GVS disabled) 5053ms -> 4276ms (-15%) 3918ms -> 4435ms (+13%) 5491ms -> 5921ms (+8%)

5f09b0c vs e9ad213 | aube/bun/pnpm | 3 scenarios | 3 runs | 500mbit/50ms | generated by Codex.

…ll-b88826

# Conflicts:
#	test/PNPM_TEST_IMPORT.md
@jdx jdx merged commit 4ed11d7 into main May 1, 2026
18 checks passed
@jdx jdx deleted the claude/vigilant-maxwell-b88826 branch May 1, 2026 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant