Skip to content

Migrate cucumber tests to e2e and vitest#6997

Open
ryancbahan wants to merge 3 commits intodelete-cucumberfrom
migrate-cucumber-to-e2e
Open

Migrate cucumber tests to e2e and vitest#6997
ryancbahan wants to merge 3 commits intodelete-cucumberfrom
migrate-cucumber-to-e2e

Conversation

@ryancbahan
Copy link
Contributor

@ryancbahan ryancbahan commented Mar 12, 2026

Summary

Re-implements the three active test scenarios that were removed with the Cucumber infrastructure in #6996:

  • Command snapshot (packages/e2e/tests/commands.spec.ts): Playwright e2e test that runs shopify commands --tree and compares against a checked-in snapshot. Catches command registration/import failures.
  • GitHub Actions pinning (packages/cli/src/cli/repo-health.test.ts): Vitest check that all third-party actions in .github/workflows/ are pinned to SHA.
  • Node deps sync (packages/cli/src/cli/repo-health.test.ts): Vitest check that shared dependencies use consistent versions across all packages.

Also adds:

  • packages/e2e/scripts/regenerate-snapshots.sh + test:regenerate-snapshots root script
  • packages/e2e/data/snapshots/commands.txt snapshot file

Test plan

  • npx vitest run packages/cli/src/cli/repo-health.test.ts — both repo health checks pass
  • pnpm -w run test:regenerate-snapshots — regenerates command snapshot
  • npx playwright test commands (from packages/e2e) — command snapshot test passes
  • npx eslint packages/cli/src/cli/repo-health.test.ts packages/e2e/tests/commands.spec.ts — clean
  • npx knip — no new findings

🤖 Generated with Claude Code

ryancbahan and others added 2 commits March 12, 2026 12:23
Re-implement the three active cucumber test scenarios that were removed
with the Cucumber infrastructure:

- Command snapshot: Playwright e2e test comparing `shopify commands --tree`
  output against a checked-in snapshot file
- GitHub Actions pinning: vitest repo health check ensuring third-party
  actions are pinned to SHA
- Node deps sync: vitest repo health check ensuring shared dependencies
  use consistent versions across packages

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add eslint-disable for no-restricted-imports and no-await-in-loop
  (repo health checks intentionally use Node stdlib directly)
- Fix id-length violations (m -> match, v -> ver)
- Add tests/tsconfig.json so eslint projectService can parse root tests
- Add eslint-disable no-restricted-imports to e2e command snapshot test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

- Move tests/repo-health.test.ts to packages/cli/src/cli/ so it lives
  in an existing vitest workspace project (no root tsconfig needed)
- Remove tests/tsconfig.json and root "." vitest workspace entry
- Add packages/e2e/scripts/* to knip ignoreBinaries to fix CI failure

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ryancbahan ryancbahan marked this pull request as ready for review March 12, 2026 18:32
@ryancbahan ryancbahan requested a review from a team as a code owner March 12, 2026 18:32
@github-actions
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@ryancbahan ryancbahan requested a review from dmerand March 12, 2026 18:33
@github-actions
Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 77.22% 14548/18839
🟡 Branches 70.91% 7217/10178
🟡 Functions 76.2% 3698/4853
🟡 Lines 78.72% 13753/17471

Test suite run success

3808 tests passing in 1465 suites.

Report generated by 🧪jest coverage report action from c90120f

@@ -0,0 +1,111 @@
/* eslint-disable no-restricted-imports, no-await-in-loop */
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to use the custom cli wrappers around fs for this test

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