test: upgrade flow testing by itsjustriley · Pull Request #3507 · Shopify/hydrogen

itsjustriley · 2026-02-25T21:39:35Z

WHY are these changes introduced?

Fixes https://github.com/Shopify/developer-tools-team/issues/1045

Two problems:

Flaky E2E tests: The existing `upgrade-flow.test.ts` was unreliable. Tests would intermittently fail in CI due to dev server flakiness (port conflicts, spawn/kill race conditions, HTTP timeouts), creating toil for releases.
Broken multi-version upgrades: When upgrading across multiple versions (e.g. 2025.4.x → 2026.1.0), the upgrade command only applied the target release's dependency list — missing intermediate bumps. It also failed with ERESOLVE errors for npm users because npm v7+ strict peer dep resolution rejects installing packages that conflict with currently-installed versions being replaced in the same command.

WHAT is this pull request doing?

Replaces `upgrade-flow.test.ts` (1313 lines) with `upgrade-e2e.test.ts`:

Removes dev server checks (the source of CI flakiness) in favor of build validation
Scaffolds real historical skeletons from git (`git archive`) instead of npm — works for older/unpublished versions and gives exact historical fidelity
Uses `inTemporaryDirectory` for cleanup and `vi.stubEnv()` for env isolation
Validates dependency/devDependency updates, `npm install` success, upgrade guide file behavior, and build success (for clean paths)
Excluded from the regular `pnpm test` suite (see `vitest.config.ts`) because it requires full git history (`fetch-depth: 0`); runs only in `test-upgrade-flow.yml`

Fixes production upgrade bugs in `upgrade.ts`:

Cumulative dependency accumulation: Multi-version upgrades now collect dependency changes from all intermediate releases (not just the target), so no intermediate bumps are skipped
`--legacy-peer-deps` for npm: Prevents ERESOLVE failures when npm v7+ strict peer dep resolution rejects packages being simultaneously replaced during upgrade

Default E2E matrix behavior:

Targets latest release (or `UPGRADE_TEST_TO`)
Selects one source version per major train from the last 365 days (highest patch/minor per major)

Explicit matrix controls:

`UPGRADE_TEST_FROM=` - Test one explicit source version
`UPGRADE_TEST_TO=` - Override target version
`UPGRADE_TEST_LAST_N=` - Test the last N source versions before target

HOW to test your changes?

E2E test scenarios

Scenario	Changelog Characteristics	What Gets Validated	Expected Output
Clean Upgrade	No `breaking: true` in target/intermediate versions, no manual `steps`	`h2 upgrade` runs, deps update, `npm install` succeeds, build succeeds	`✓ X → Y`
Manual Steps	Target or intermediate version includes `steps`, no breaking	`h2 upgrade` runs, deps update, `npm install` succeeds, guide file/content validated, build skipped	`✓ X → Y [manual steps required, build skipped]`
Breaking Changes	Target or intermediate version includes `breaking: true`	`h2 upgrade` runs, deps update, `npm install` succeeds, guide behavior validated if steps exist, build skipped	`✓ X → Y [breaking changes, build skipped]`
Missing Packages	Required target package versions are not on npm	Fails fast before scaffold/upgrade	`Required packages not yet published to npm: ...`
Incomplete Changelog	Missing/inaccurate dependency transitions	Install/build fails with maintainer-facing guidance	`Changelog may be incomplete or inaccurate for this upgrade path`
Default Matrix	No `UPGRADE_TEST_FROM`, no `UPGRADE_TEST_LAST_N`	Tests highest patch/minor per major in last 365 days → target	`Testing N major versions from last 365 days: A → B`
LAST_N Matrix	`UPGRADE_TEST_LAST_N` is set	Tests all selected source paths to one target	`Testing last N: ...` and one result per path
Hash Mismatch/Unreachable	`hash` is missing/unreachable or does not match skeleton	Warning + fallback git search	Warning about hash fallback

Note: changelog schema validation and exhaustive per-dependency assertions are covered in unit tests. This E2E focuses on end-to-end upgrade execution, install success, and build/guide behavior.

Tophatting commands

# Run from repo root. The test is excluded from pnpm test — run it directly:

# 1) Clean non-breaking adjacent path (also validates --legacy-peer-deps fix)
pnpm --dir packages/cli exec cross-env UPGRADE_TEST_FROM=2025.10.0 UPGRADE_TEST_TO=2025.10.1 vitest run --config vitest.e2e.config.ts

# 2) Multi-version path (validates cumulative dep accumulation)
pnpm --dir packages/cli exec cross-env UPGRADE_TEST_FROM=2025.7.0 UPGRADE_TEST_TO=2026.1.0 vitest run --config vitest.e2e.config.ts

# 3) Manual-steps path: guide expected, build skipped
pnpm --dir packages/cli exec cross-env UPGRADE_TEST_FROM=2025.4.2 UPGRADE_TEST_TO=2025.5.1 vitest run --config vitest.e2e.config.ts

# 4) LAST_N matrix to one target (runs all selected paths)
pnpm --dir packages/cli exec cross-env UPGRADE_TEST_TO=2025.10.1 UPGRADE_TEST_LAST_N=3 vitest run --config vitest.e2e.config.ts

# 5) Default matrix behavior (one source per major train from last 365 days -> latest target)
pnpm --dir packages/cli exec vitest run --config vitest.e2e.config.ts

Verify CI

Modify `docs/changelog.json` (e.g. whitespace)
Push to branch
Confirm `test-upgrade-flow.yml` passes and `Unit tests` passes

Post-merge steps

None required.

Checklist

I've read the Contributing Guidelines
I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've added a changeset
I've added tests to cover my changes
I've added or updated the documentation

shopify · 2026-02-25T21:40:52Z

Oxygen deployed a preview of your upgrade-flow-tests-new branch. Details:

Storefront	Status	Preview link	Deployment details	Last update (UTC)
Skeleton (skeleton.hydrogen.shop)	✅ Successful (Logs)	Preview deployment	Inspect deployment	March 18, 2026 9:15 PM

Learn more about Hydrogen's GitHub integration.

binks-code-reviewer · 2026-03-02T14:26:26Z

🤖 Code Review · #projects-dev-ai for questions
React with 👍/👎 or reply — all feedback helps improve the agent.

✅ Complete - 1 findings

📋 History

✅ 1 findings → ✅ 1 findings → ✅ 1 findings

The previous upgrade-flow.test.ts used mocked file system operations and upgrade logic, making it prone to false positives that didn't reflect real upgrade behavior. This replaces it with upgrade-e2e.test.ts, which scaffolds real skeleton projects from git history and runs the full upgrade command end-to-end. Key design decisions: - Uses git archive to extract skeleton templates at exact historical commits, ensuring the scaffolded project matches what merchants actually had at that version rather than a synthetic approximation - Builds a version matrix from the changelog (one upgrade path per major CalVer train per year, within a 365-day lookback window) so coverage scales automatically as new releases land - Skips build validation when manual steps or breaking changes are present, since developers must apply those steps before a project will build - Validates dependency versions in package.json after upgrade to catch changelog inaccuracies early Co-authored-by: Claude <claude@anthropic.com>

…er-deps consistently Three related fixes to the upgrade command's dependency management: 1. Accumulate removeDependencies and removeDevDependencies across all intermediate releases when upgrading across multiple versions. Previously only the target release's removals were applied, causing packages removed in intermediate releases (e.g. @shopify/remix-oxygen) to remain installed. 2. Add --legacy-peer-deps to npm install. npm v7+ strict peer dep resolution rejects installing packages that conflict with currently-installed versions, even when those old versions are being replaced in the same command. --legacy-peer-deps reverts to npm v6 behavior, which resolves correctly for upgrade scenarios where peer dep versions are changing. pnpm/yarn handle this without special flags. 3. Add --legacy-peer-deps to npm uninstall for the same reason. npm v7+ runs full peer dep resolution during uninstall too, which rejects the operation when old and new peer dep ranges conflict (e.g. vite@^5 from an old scaffold vs vite@^6 required by the new mini-oxygen). Co-authored-by: Claude <claude@anthropic.com>

Some changelog entries have hashes that point to blob objects rather than commits (e.g. all 2025.1.x versions), and CI's shallow clone can't find them via git log search either. Two compounding problems required a two-part fix: 1. resolveSkeletonCommit now walks back through all releases in the same major train (YYYY.M) rather than one step, stopping at the first valid commit. A major-train boundary guard (getMajorVersion) prevents silently crossing year boundaries, which would scaffold from a structurally different release and compromise test fidelity. The repeated hash-validate + git-search-fallback logic is extracted into resolveReleaseCommit, making the walk-back loop body a one-liner. 2. The test loop now catches ScaffoldNotFoundError (a typed class replacing the generic Error throw) and skips that upgrade path with a warning rather than aborting the full matrix. A testedCount guard ensures we fail loudly if no paths could be tested at all, preventing silent false greens. Co-authored-by: Claude <claude@anthropic.com>

Replace shell-interpolated execAsync calls with shell-free execFileAsync (promisify(execFile)) throughout the git commit search helpers, eliminating a shell injection vector where release.hash from changelog.json was interpolated into sh -c strings. Additional fixes surfaced during consensus review: - Replace findGitRepoRoot (fragile relative-path probe) with getRepoRoot using git rev-parse --show-toplevel for canonical repo root resolution - Remove git fetch origin from test execution (network call, mutates local state, already present from CI checkout) - Consolidate the two independent hardcoded repoRoot join(cwd, '../../') callsites in resolveSkeletonCommit and extractSkeletonTemplate to use getRepoRoot() with an explicit error when outside a git repo - Add console.warn when resolveSkeletonCommit falls back to a prior version's skeleton, making the substitution visible in test output - Remove unreachable toBeDefined() assertion from validateDependencyVersion (all callers guard with if (actualVersion) before calling) - Replace sequential npm view loop in arePackagesPublished with Promise.allSettled for concurrent package existence checks - Throw on empty UPGRADE_TEST_LAST_N result instead of silently returning an empty matrix (which would appear as a passing no-op test run) - Extend test-upgrade-flow.yml trigger paths to include upgrade*.ts so changes to the test or command files re-run CI without a changelog edit - Add changeset for @shopify/cli-hydrogen patch (runUpgrade/getChangelog are now exported as public API) Co-authored-by: Claude <claude@anthropic.com>

The upgrade E2E tests scaffold historical skeleton versions by calling git show <commit>:templates/skeleton/package.json and git log across the full commit history. actions/checkout defaults to depth=1 (shallow clone), which means all historical commits are unavailable — causing every scaffold attempt to fail and the entire test suite to report "no upgrade paths could be tested". fetch-depth: 0 fetches the complete history so git archive and git log can reach the release commits referenced in changelog.json. Co-authored-by: Claude <claude@anthropic.com>

The upgrade-e2e.test.ts was running in the regular pnpm test / turbo test suite, which uses a shallow git clone in CI. All scaffolding attempts failed with ScaffoldNotFoundError, causing the Unit tests CI job to fail. The test belongs exclusively in test-upgrade-flow.yml which already has fetch-depth: 0. - Add **/*.e2e.test.ts to vitest test.exclude (spreading configDefaults to preserve defaults like **/dist/**) so e2e tests only run in their dedicated CI workflow - Replace exec('mv', ...) with rename() from node:fs/promises for cross-platform correctness and consistency with the rest of the file - Remove misleading "defense-in-depth" comment and redundant toBeDefined() check from validateDependencyVersion — TypeScript types already guarantee the parameter is string; the comment implied undefined was possible when it isn't - Update changeset description to accurately describe the user-facing changes: intermediate dep accumulation and --legacy-peer-deps for npm

…uns via dedicated workflow

The pattern **/*.e2e.test.ts uses a dot separator and does not match upgrade-e2e.test.ts which uses a hyphen. Changed to **/*-e2e.test.ts so the file is actually excluded from the regular unit test run.

…conflict vitest's exclude patterns take precedence over CLI file filters, so the **/*-e2e.test.ts exclusion in vitest.config.ts also blocked the dedicated test-upgrade-flow.yml workflow from running the test. Solution: a separate vitest.e2e.config.ts that explicitly includes src/**/*-e2e.test.ts (with its own timeout). The dedicated workflow now uses --config vitest.e2e.config.ts directly, bypassing the unit test exclusion entirely. Regular pnpm test continues to use vitest.config.ts which still excludes **/*-e2e.test.ts.

A change to vitest.e2e.config.ts (e.g. adjusting timeout or include pattern) would not previously trigger the workflow, meaning a broken config change would only be caught when changelog.json or upgrade*.ts next changed.

…e paths The post-upgrade npm install validation step now uses --legacy-peer-deps only for upgrade paths with manual steps or breaking changes, where the project is in an intermediate state with expected peer dep conflicts from old-era transitive dependencies. Clean upgrade paths retain strict peer dep resolution, which catches incomplete or inaccurate changelog entries that would otherwise be masked. This mirrors the production upgrade command's use of --legacy-peer-deps while being intentionally stricter for clean paths as a changelog accuracy check. Co-authored-by: Claude <noreply@anthropic.com>

kdaviduik

some comments/suggestions but I'd be happy to ship this and do any of these as a follow-up. imo not an urgent follow-up (we have bigger fish to fry and real issues to fix!). LGTM 🚀

itsjustriley · 2026-03-19T13:21:48Z

cookie tests fail on main, all relevant tests pass.

itsjustriley changed the title ~~fix: upgrade flow testing~~ test: upgrade flow testing Feb 26, 2026

itsjustriley force-pushed the upgrade-flow-tests-new branch 2 times, most recently from 9d5324c to 6b16405 Compare February 26, 2026 11:33

itsjustriley marked this pull request as ready for review March 2, 2026 14:11

itsjustriley requested a review from a team as a code owner March 2, 2026 14:11

This comment has been minimized.

Sign in to view

binks-code-reviewer Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread packages/cli/src/commands/hydrogen/upgrade-e2e.test.ts

binks-code-reviewer Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread packages/cli/src/commands/hydrogen/upgrade-e2e.test.ts

binks-code-reviewer Bot reviewed Mar 2, 2026

View reviewed changes

Comment thread packages/cli/src/commands/hydrogen/upgrade-e2e.test.ts

itsjustriley requested review from andguy95, fredericoo and kdaviduik March 2, 2026 16:34

itsjustriley marked this pull request as draft March 4, 2026 16:53

itsjustriley force-pushed the upgrade-flow-tests-new branch from 0eb58c7 to 737d1d2 Compare March 4, 2026 16:56

itsjustriley closed this Mar 6, 2026

itsjustriley reopened this Mar 6, 2026

itsjustriley force-pushed the upgrade-flow-tests-new branch from 62a9d08 to ec2fc47 Compare March 9, 2026 16:02

itsjustriley mentioned this pull request Mar 11, 2026

fix: h2 upgrade command handles cumulative dependency removal #3527

Merged

5 tasks

itsjustriley force-pushed the upgrade-flow-tests-new branch 3 times, most recently from ce3c19c to 20c667c Compare March 18, 2026 16:25

itsjustriley and others added 3 commits March 18, 2026 14:39

itsjustriley force-pushed the upgrade-flow-tests-new branch from c476055 to 98ed36d Compare March 18, 2026 18:40

itsjustriley and others added 3 commits March 18, 2026 15:31

itsjustriley and others added 5 commits March 18, 2026 16:08

docs(test): document that upgrade-e2e requires full git history and r…

af8f81f

…uns via dedicated workflow

fix(test): correct vitest exclude glob pattern for e2e tests

600cee7

The pattern **/*.e2e.test.ts uses a dot separator and does not match upgrade-e2e.test.ts which uses a hyphen. Changed to **/*-e2e.test.ts so the file is actually excluded from the regular unit test run.

ci: trigger upgrade flow tests when vitest e2e config changes

574d15c

A change to vitest.e2e.config.ts (e.g. adjusting timeout or include pattern) would not previously trigger the workflow, meaning a broken config change would only be caught when changelog.json or upgrade*.ts next changed.

itsjustriley marked this pull request as ready for review March 18, 2026 21:19

kdaviduik approved these changes Mar 19, 2026

View reviewed changes

itsjustriley merged commit 2faecb2 into main Mar 19, 2026
30 of 32 checks passed

itsjustriley deleted the upgrade-flow-tests-new branch March 19, 2026 13:22

shopify-github-actions-access Bot mentioned this pull request Mar 19, 2026

[ci] release 2026.1.3 #3584

Merged

This was referenced Apr 10, 2026

hydrogen/Bump cli-hydrogen to 11.1.14 Shopify/cli#7246

Merged

hydrogen/Bump cli-hydrogen to 11.1.14 (main) Shopify/cli#7234

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: upgrade flow testing#3507

test: upgrade flow testing#3507
itsjustriley merged 11 commits intomainfrom
upgrade-flow-tests-new

itsjustriley commented Feb 25, 2026 •

edited

Loading

Uh oh!

shopify Bot commented Feb 25, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

Uh oh!

binks-code-reviewer Bot commented Mar 2, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

kdaviduik left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

itsjustriley commented Mar 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

itsjustriley commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

HOW to test your changes?

E2E test scenarios

Tophatting commands

Verify CI

Post-merge steps

Checklist

Uh oh!

shopify Bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

binks-code-reviewer Bot commented Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kdaviduik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

itsjustriley commented Mar 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

itsjustriley commented Feb 25, 2026 •

edited

Loading

shopify Bot commented Feb 25, 2026 •

edited

Loading

binks-code-reviewer Bot commented Mar 2, 2026 •

edited

Loading