Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not prompt to install chromatic script during E2E builds #941

Conversation

tevanoff
Copy link
Contributor

@tevanoff tevanoff commented Mar 6, 2024

The CLI will prompt to install a chromatic script if one does not already exist in package.json. This will be the wrong command for E2E builds and may be missing necessary env vars. This disables that for E2E builds for the time being.

📦 Published PR as canary version: 11.0.5--canary.941.8178177052.0

✨ Test out this PR locally via:

npm install chromatic@11.0.5--canary.941.8178177052.0
# or 
yarn add chromatic@11.0.5--canary.941.8178177052.0

@tevanoff tevanoff marked this pull request as ready for review March 6, 2024 18:19
@tevanoff tevanoff requested a review from skitterm March 6, 2024 18:19
Copy link
Member

@skitterm skitterm left a comment

Choose a reason for hiding this comment

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

Looks good!

Comment on lines +63 to +65
export function isE2EBuild(options: Options) {
return options.playwright || options.cypress;
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice. We could use this for the "flag the project as e2e" thing in the future (if we ever feel the need to handle projects switching from storybook-e2e or vice versa).

Comment on lines +716 to +723
it('does not propmpt you to add a script to your package.json for E2E builds', async () => {
const ctx = getContext(['--project-token=asdf1234', '--playwright']);
await runAll(ctx);
const spy = vi.spyOn(checkPackageJson, 'default');
expect(spy).not.toHaveBeenCalled();
expect(confirm).not.toHaveBeenCalled();
});

Copy link
Member

Choose a reason for hiding this comment

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

Nice test.

@tevanoff tevanoff added release Auto: Create a `latest` release when merged patch Auto: Increment the patch version when merged labels Mar 6, 2024
@tevanoff tevanoff added this pull request to the merge queue Mar 6, 2024
Merged via the queue into main with commit ff1d35b Mar 6, 2024
24 of 26 checks passed
@tevanoff tevanoff deleted the todd/ap-4260-cli-prompts-user-to-install-wrong-chromatic-command branch March 6, 2024 20:12
@ghengeveld
Copy link
Member

🚀 PR was released in v11.0.4 🚀

@ghengeveld ghengeveld added the released Verdict: This issue/pull request has been released label Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Auto: Increment the patch version when merged release Auto: Create a `latest` release when merged released Verdict: This issue/pull request has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants