test: port three test-only pnpm tests + retriage two#481
Conversation
Greptile SummaryPorts three pnpm-parity tests — a workspace The test implementations are well-structured and consistent with the codebase's existing patterns. Confidence Score: 5/5Safe to merge — changes are confined to test additions, a new offline fixture, and triage-doc updates with no production code touched. No P0 or P1 issues found. All new tests follow established bats/assertion patterns in the repo; the new registry fixture is structurally consistent with existing ones; the triage-doc reclassifications are supported by cited empirical evidence. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "docs(test): update triage doc for ported..." | Re-trigger Greptile |
Benchmark changesPublic ratios: warm installs vs Bun 7x -> 5x; warm installs vs pnpm 11x -> 13x.
6072789 vs 56a5651 | aube/bun/pnpm | 3 scenarios | 3 runs | 500mbit/50ms | generated by Codex. |
af9e885 to
75aaf92
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 75aaf92. Configure here.
…s.ts:108) Adds the @pnpm.e2e/aube-test-failing-install fixture (1.0.0, install: exit 1) under test/registry/storage/, mirroring pnpm's package-that-cannot-be- installed shape. Asserts state-not-written-on-failure semantics — first install fails, second install still fails, removing the broken dep restores success — without the literal pnpm dir-removal assertion that doesn't translate to aube's CAS architecture. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workspace install with a readPackage hook injecting a peerDep into is-odd, then aube remove is-even from the sub-project. The survivor's snapshot must retain the injected peerDep — proves the chained install triggered by remove wires the readPackage host the same way aube install does. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Network-gated port asserting aube installs cleanly when a git URL's fragment contains a forward slash (e.g. branch/with-slash). Uses pnpm's own upstream fixture (github.com/pnpm-e2e/simple-pkg.git#branch/with- slash) and pins the resolved committish in the lockfile assertion. Confirms the parser at aube-lockfile/src/lib.rs:645 routes slash-bearing fragments via the "" fallback branch, and the resolver at aube-resolver/src/local_source.rs:242 walks the resulting committish through git ls-remote. Gated behind AUBE_NETWORK_TESTS=1 (same convention as test/pnpm_update_slow.bats). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Mark hooks.ts:580 (readPackage during remove), misc.ts:567 (git fragment with slash), and lifecycleScripts.ts:108 (rollback) as ported. - Retriage hooks.ts:68 (readPackage returning undefined) from test-only to won't-support: empirical check shows aube's IPC shim falls back to the original pkg, not the documented pkg-missing error path. Move to Tier 3 (existing skip in pnpm_install_hooks.bats already documents it). - Retriage lifecycleScripts.ts:179/200 (verify-deps + preinstall sub- aube calls) from test-only to support (real bug). With verifyDepsBeforeRun=error the inner aube run from a preinstall script fails on install-state-not-found; with mode=install the inner call deadlocks on the project lock. Aube fix: skip ensure_installed when npm_lifecycle_event is set. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
75aaf92 to
6072789
Compare
|
Addressed Cursor Bugbot's high-severity finding: the rollback test was using Re-running the test with the fix surfaced a second issue: the Test now fails for the correct reason (install-script Squashed into the original rollback commit via Written with Claude. |
Summary
Ports three of the five test-only pnpm tests called out in the triage brief, retriages the other two with empirical evidence:
misc.ts:567(git URL with hash containing slash)test/pnpm_install_misc_slow.bats(network-gated, new file)hooks.ts:68(readPackage returning undefined fails)pnpm_install_hooks.bats:310; triage retriaged to won't-supporthooks.ts:580(readPackage during remove in workspace)test/pnpm_install_hooks.batslifecycleScripts.ts:108(rollback-on-build-failure)test/lifecycle_scripts.bats+ new@pnpm.e2e/aube-test-failing-installfixturelifecycleScripts.ts:179, 200(verify-deps + preinstall sub-aube)Items not ported (retriaged with evidence)
Item 2 (
hooks.ts:68): triage said aube errors withreadPackage response missing pkg. Empirically, the IPC shim atcrates/aube/src/pnpmfile.rs:629falls back to the original pkg viaif (out && typeof out === 'object') result = out;and writes back{ id, pkg: <original> }. The Rust side atpnpmfile.rs:809seespkgpresent and continues with the original manifest — install succeeds rather than fails. The existing skipped test atpnpm_install_hooks.bats:310(added in a previous PR) already documents this divergence correctly. Triage doc updated to move this to Tier 3 (won't-support / divergence).Item 5 (
lifecycleScripts.ts:179, 200): triage said aube has the parent-set recursion guard, but the only "parent-set" mechanism atrun.rs:536preservesINIT_CWD, not lifecycle context. Empirically:verifyDepsBeforeRun=error+preinstall: aube run sayHello→ inneraube runexits withdependencies need install before run: install state not found(state isn't written until linking finishes).verifyDepsBeforeRun=install→ inneraube runtriggersensure_installed→install::run, which deadlocks on the project lock the outer install holds (Waiting for another aube process to finish in this project..., observed via 60s timeout).Per the brief: "If porting surfaces a real recursion bug, STOP — don't fight the bug." Aube fix needed: skip
ensure_installedwhennpm_lifecycle_eventis set in the env (matches npm/pnpm's "no verify-deps inside lifecycle scripts" contract). Tracked in the retriaged entry in the triage doc.New offline fixture
test/registry/storage/@pnpm.e2e/aube-test-failing-install/(1.0.0,install: exit 1) — minimal hand-built tarball + packument JSON, mirrors the structure of the existing@pnpm.e2e/install-script-examplefixture. Used by the rollback test only.References
test/PNPM_TEST_IMPORT.md(see PR docs(test): triage every pnpm-test-import gap with explicit support decisions #471 for triage context).test/pnpm_update_slow.bats.Test plan
cargo build --workspacecargo clippy --all-targets -- -D warnings(zero warnings)cargo fmt --check(clean — no Rust source changes)mise run test:bats test/pnpm_install_hooks.bats— 21 passing (3 documented skips), including the new readPackage-during-remove testmise run test:bats test/lifecycle_scripts.bats— 34 passing, including the new rollback testAUBE_NETWORK_TESTS=1 ./test/bats/bin/bats test/pnpm_install_misc_slow.bats— passes (network required)Note
Low Risk
Test-only changes add new bats coverage (including a network-gated case) plus an offline fixture; no production Rust code paths are modified, so risk is limited to CI/test stability and reliance on external GitHub availability for the slow test.
Overview
Adds three new pnpm parity ports to the bats suite: a network-gated
misc.ts:567install-from-git regression test (newpnpm_install_misc_slow.bats), a workspaceaube removepath hook regression test inpnpm_install_hooks.bats, and a lifecycle rollback/retry contract test inlifecycle_scripts.batsbacked by a new always-failing offline fixture@pnpm.e2e/aube-test-failing-install.Updates
PNPM_TEST_IMPORT.mdto mark the newly-ported items as done and to retriage two previously “test-only” gaps:readPackagereturningundefinedis now documented as an intentional aube divergence, and theverify-deps-before-run+ lifecycle-subcommand case is reclassified as a real aube bug requiring a fix before porting.Reviewed by Cursor Bugbot for commit 6072789. Bugbot is set up for automated code reviews on this repo. Configure here.