Repository navigation
fix(repo): install npm-alias closure edges from their packed tarball in the e2e bake - #237
Merged
Merged
Conversation
…in the e2e bake
The stryker-js manifest names the vm runner through a workspace alias
("@systemfsoftware/stryker-js-vm-runner": "workspace:@systemfsoftware/stryker-js-vitest-runner@^"),
which pnpm pack rewrites to npm:@systemfsoftware/stryker-js-vitest-runner@^<version>.
The bake ran `npm install /packs/*.tgz`. npm places the runner tarball under its
own name, so nothing fills the alias slot and npm resolves the npm: spec from the
registry. On main that silently downloaded the published runner (then copied the
packed one over it); on the 18.0.0 release PR the runner is 9.0.0, which the
registry does not have, and every e2e job failed with ETARGET
installClosure (stryker-e2e-core) now plans the install from the packed
manifests: every tarball plus `<alias>@file:<target tarball>` for each npm-alias
edge between closure members, and a refusal (UnpackedWorkspaceDependency) for an
edge to a workspace package the closure did not pack. The harness passes the plan
to bake-fixtures.sh, whose copy-over-the-alias step is gone. The packed tarball
lookup now matches <scope>-<name>-<semver>.tgz exactly; the prefix match let
@systemfsoftware/stryker-js pick up a sibling's tarball
install-closure.integration.test.ts packs a closure with the same alias shape and
installs the plan offline with an empty cache and an unreachable registry, then
requires file: provenance for every workspace package in the lockfile
Verdict-Semantics: unchanged
…its test Verdict-Semantics: unchanged
systemfsoftware-maker
changed the title
…e the e2e bake installs Review of the closure install plan found three edges it let through to the registry or to the wrong tarball. A fixture manifest naming a workspace package is resolved by the fixture's own npm install before the closure specs run, so the plan now refuses it (FixtureNamesWorkspacePackage). Two alias edges with one name and different targets left npm to keep the last spec, so the plan refuses them (ConflictingAliasTargets). Optional peers npm never installs are no longer edges. The plan runs only when a fixture needs baking, and the packed-manifest schemas move to install-closure.schema.ts Verdict-Semantics: unchanged
![kiro-systemf[bot]](https://avatars.githubusercontent.com/u/172220390?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why every e2e job fails on the release PR
CI run 37744072050 on #236 (cb7dde2) fails all five e2e jobs in the fixture bake with
npm error notarget No matching version found for @systemfsoftware/stryker-js-vitest-runner@^9.0.0.The manifest that asks for the runner:
packages/stryker-js/package.json:49(the same line on main 1910752 and on cb7dde2):That's a pnpm workspace alias, not a
catalog:spec.pnpm packrewrites it to an npm alias. On main the packed CLI manifest says"@systemfsoftware/stryker-js-vm-runner": "npm:@systemfsoftware/stryker-js-vitest-runner@^8.1.1". On cb7dde2 the runner is 9.0.0, so it says^9.0.0.Why the packed copy isn't used:
test/e2e/tests/__fixtures__/bake-fixtures.sh:10runsnpm install /packs/*.tgz. npm installs the runner tarball under its own name (stryker-js-vitest-runner). Annpm:alias edge is a separate dependency under a different name, and npm resolves it from the registry even when the target's tarball is in the same install. The closure itself is complete: the runner is an entry package, so its tarball was always in/packs. What fails is the install, which assumes npm matches every edge by package name.On main the registry has
8.1.1, so the install succeeds. Lines 11-18 thencp -Rthe packed runner over the alias folder, so the runner code under test was the workspace build. Its dependency tree, though, came from the published 8.1.1 manifest, and every bake pulled that from the registry. On the release tree^9.0.0isn't on the registry and never will be, so the install fails before the copy runs.The bump isn't the cause: pnpm-release-management wrote 9.0.0, and
^9.0.0is the correct range.Fix
test/e2e-core/src/install-closure.workflow.ts(new):installClosureplans the bake's install from the packed manifests and the fixture manifests. The plan is every packed tarball, plus<alias>@file:<target tarball>for eachnpm:alias edge between closure members. Edges are what npm installs:dependencies,optionalDependencies, and peers not marked optional inpeerDependenciesMeta. The plan refuses, naming the edge, instead of reaching the registry, in three cases:UnpackedWorkspaceDependency: a member's edge targets a workspace package the closure didn't pack.ConflictingAliasTargets: one alias name would install two different packages. npm keeps whichever spec comes last.FixtureNamesWorkspacePackage: a fixture manifest names a workspace package. The fixture's ownnpm installruns before the closure specs, so it would resolve that package from the registry.test/e2e-core/src/install-closure.schema.ts(new): the packed-manifest and fixture-manifest schemas.test/e2e/src/Harness/fixture-cache.service.ts: on a cold bake only, gets the list of workspace package names (pnpm ls -r --depth -1 --json), reads each tarball'spackage/package.jsonand each fixture manifest, runs the plan, and passes the specs to the bake script as arguments. The tarball lookup now matches<scope>-<name>-<semver>.tgzexactly. It used to match a name prefix, so@systemfsoftware/stryker-jscould pick up…-stryker-js-vitest-runner-*.tgz. TheUnreadableVersioncase this made unreachable is deleted.test/e2e/tests/__fixtures__/bake-fixtures.sh: installs"$@". The alias copy workaround is removed.test/e2e-core/tests/install-closure.integration.test.ts, which runs inpnpm test.npm install --package-lock-only --offline, an empty cache and an unreachable registry, then requires every workspace@systemfsoftware/*lockfile entry to resolve tofile:.test/e2e/README.mdanddocs/solutions/build-errors/e2e-bake-resolved-an-npm-alias-edge-from-the-registry.md.No changeset: only private test packages change (
@systemfsoftware/stryker-e2e,@systemfsoftware/stryker-e2e-core).Evidence
The test against main's install rule (1910752,
bake-fixtures.sh:10, tarballs only). This is the current test file, run in a throwaway checkout of 1910752 with the branch's workflow module minus its alias-spec line, so the plan is exactly main's tarball list:The same test on this branch:
Test Files 1 passed (1)/Tests 5 passed (5).The e2e bake on #236's tree (a throwaway checkout of cb7dde2 with this branch's diff applied, the baked cache removed first,
pnpm lgtm:up, thenpnpm test:e2e -- tests/vm-vitest.e2e.test.ts). Global setup baked all 7 fixtures in this run (the entry mtimes fall after the run start). Raw output:The only
@systemfsoftware/*package that still comes from the registry is@systemfsoftware/effect-cell-types@12.0.0. It isn't a workspace package (it's not one of the 23 inpnpm ls -r; this repo pins it through the catalog at^12.0.0), so the closure has no tarball for it.