Skip to content

add SHOPIFY_CLI_ENV=development for test commands#6998

Merged
phyllis-sy-wu merged 1 commit intomainfrom
psyw-0312-add-SHOPIFY_CLI_ENV_development-for-dev-test
Mar 19, 2026
Merged

add SHOPIFY_CLI_ENV=development for test commands#6998
phyllis-sy-wu merged 1 commit intomainfrom
psyw-0312-add-SHOPIFY_CLI_ENV_development-for-dev-test

Conversation

@phyllis-sy-wu
Copy link
Copy Markdown
Contributor

@phyllis-sy-wu phyllis-sy-wu commented Mar 12, 2026

WHY are these changes introduced?

Some unit tests fail when SHOPIFY_CLI_ENV=development is not set. Specifically, four tests in the removeDuplicatedPlugins suite in base-command.test.ts depend on BaseCommand.init() skipping registerCleanBugsnagErrorsFromWithinPlugins (which only runs when not in development mode). Without this skip, that function runs against mock plugins that have no root property, throws before removeDuplicatedPlugins can execute, and the assertions fail.

Relevant discussion: https://shopify.slack.com/archives/C05E3BDFDRB/p1773343132505319

Context

There are three ways to run unit tests locally:

  1. dev test
    • Defined in dev.yml.
    • For internal Shopify engineers.
    • SHOPIFY_CLI_ENV=development is already set in dev.yml, so tests pass after dev up.
  2. pnpm test
    • Defined in package.json.
    • For both external contributors and internal engineers.
    • Previously did not set SHOPIFY_CLI_ENV=development, so four tests failed unless the caller exported the variable manually.
  3. Direct invocationpnpm vitest run
    • Same issue as above; requires manually setting SHOPIFY_CLI_ENV=development.

WHAT is this pull request doing?

  • Sets SHOPIFY_CLI_ENV: 'development' in vite.config.ts under test.env, so the variable is applied to all vitest invocations (pnpm test, pnpm vitest run, and CI) on all platforms — without requiring a shell prefix or additional dependencies like cross-env.
  • Removes the now-redundant test:unit script. After cucumber tests were removed, test and test:unit became identical (pnpm vitest run). Keeping both caused confusion with no benefit.
  • Updates .github/workflows/tests-manual.yml to reference pnpm test instead of the removed pnpm test:unit.

How to test your changes?

Run the commands below on different branches:

dev up
unset SHOPIFY_CLI_ENV
pnpm test # or dev test
# Output

On main - reproduce the failure:
Output: 4 tests fail in base-command.test.ts > removeDuplicatedPlugins

On this branch - verify the fix:
Output: all tests pass

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Copy Markdown
Contributor Author

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

@phyllis-sy-wu phyllis-sy-wu marked this pull request as ready for review March 12, 2026 22:46
@phyllis-sy-wu phyllis-sy-wu requested a review from a team as a code owner March 12, 2026 22:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 12, 2026

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements 82.21% 14927/18157
🟡 Branches 74.64% 7392/9904
🟢 Functions 81.36% 3767/4630
🟢 Lines 82.61% 14114/17086

Test suite run success

3910 tests passing in 1502 suites.

Report generated by 🧪jest coverage report action from 193147e

Comment thread package.json Outdated
Copy link
Copy Markdown
Contributor

@ryancbahan ryancbahan left a comment

Choose a reason for hiding this comment

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

nice job! let's just make sure all the package.json commands that need this env var have it

@phyllis-sy-wu phyllis-sy-wu changed the title add SHOPIFY_CLI_ENV=development for dev test add SHOPIFY_CLI_ENV=development for test commands Mar 13, 2026
Copilot AI review requested due to automatic review settings March 18, 2026 20:47
@phyllis-sy-wu phyllis-sy-wu force-pushed the psyw-0312-add-SHOPIFY_CLI_ENV_development-for-dev-test branch from 13bbcda to cdbf4d0 Compare March 18, 2026 20:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates local test entrypoints to run under the development Shopify CLI environment so behavior/config matches dev test.

Changes:

  • Prefixes the root pnpm test script with SHOPIFY_CLI_ENV=development when running vitest.
  • Adjusts dev.yml’s test command to call pnpm run test (instead of test:unit).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
package.json Updates root test script to run vitest with SHOPIFY_CLI_ENV=development (and removes test:unit).
dev.yml Switches dev test to invoke pnpm run test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread package.json
Comment thread package.json Outdated
@phyllis-sy-wu phyllis-sy-wu force-pushed the psyw-0312-add-SHOPIFY_CLI_ENV_development-for-dev-test branch from cdbf4d0 to 193147e Compare March 18, 2026 21:25
@phyllis-sy-wu phyllis-sy-wu added the #gsd:49408 Agentic app validation label Mar 18, 2026
@phyllis-sy-wu phyllis-sy-wu added this pull request to the merge queue Mar 19, 2026
Merged via the queue into main with commit 0318e7b Mar 19, 2026
29 checks passed
@phyllis-sy-wu phyllis-sy-wu deleted the psyw-0312-add-SHOPIFY_CLI_ENV_development-for-dev-test branch March 19, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

#gsd:49408 Agentic app validation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants