chromaui / addon-visual-tests

Visual Tests addon for Storybook
MIT License
33 stars 2 forks source link

git is required for visual test addon via chromatic cli #126

Closed weeksling closed 11 months ago

weeksling commented 11 months ago

This adds an error state to the panel, disables the run button, and shows a notification if the addon is run in a project without git configured.

To test this:

Note: this currently stops the gitObservable loop from occurring and requires the server to be restarted after fixing the issue. Added language in the notification to prompt the user to restart storybook after fixing.

Note: This does not handle build errors once the gitInfo can be retrieved. The pr this branches off of should handle build-level errors including the git one commit issue.

📦 Published PR as canary version: 0.0.99--canary.126.8f69032.0
:sparkles: Test out this PR locally via: ```bash npm install @chromaui/addon-visual-tests@0.0.99--canary.126.8f69032.0 # or yarn add @chromaui/addon-visual-tests@0.0.99--canary.126.8f69032.0 ```
linear[bot] commented 11 months ago
AP-3618 Git is required for visual test addon via chromatic CLI

**Action Item**: handle git errors from the CLI and display this design in the addon: [Panel - Visual testing in Storybook (Figma)](https://www.figma.com/file/GFEbCgCVDtbZhngULbw2gP/Visual-testing-in-Storybook?type=design&node-id=1898-563208&mode=design&t=M8C1zgw19KVEajF9-0) **Feedback:** The addon uses Chromatic under the hood which requires a Git remote with at least one commit to work. The main issue with this is that the error isn't handled with proper help and since I don't necessarily know the Chromatic CLI is being used under the hood, I wouldn't know where to go for help. At the least, the error should be handled with instructions but ideally a Git commit isn't required for the visual test addon to work (not sure whether this is feasible). ## Reproduction 1. Init SB with Vite + React TS in an empty dir 2. Add the visual test addon 3. Start Storybook ← This errors with: ``` info => Starting manager.. Unhandled promise rejection: Error: Command failed with exit code 1: git config --get remote.origin.url at Eir (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:197:57) at /Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:197:8411 at processTicksAndRejections (node:internal/process/task_queues:96:5) at async Ki (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:287:45) at async NM (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:287:530) at async Object.N3i (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:1298:1359) at async c (/Users/zol/Git/test-visual-testing-addon/node_modules/chromaui/addon-visual-tests/dist/index.js:12:1250) { shortMessage: 'Command failed with exit code 1: git config --get remote.origin.url', command: 'git config --get remote.origin.url', escapedCommand: 'git config --get remote.origin.url', exitCode: 1, signal: undefined, signalDescription: undefined, stdout: '', stderr: '', all: '', failed: true, timedOut: false, isCanceled: false, killed: false } Error: Command failed with exit code 1: git config --get remote.origin.url at Eir (./node_modules/chromatic/dist/chunk-J3MH667B.js:197:57) at ./node_modules/chromatic/dist/chunk-J3MH667B.js:197:8411 at processTicksAndRejections (node:internal/process/task_queues:96:5) at async Ki (./node_modules/chromatic/dist/chunk-J3MH667B.js:287:45) at async NM (./node_modules/chromatic/dist/chunk-J3MH667B.js:287:530) at async Object.N3i (./node_modules/chromatic/dist/chunk-J3MH667B.js:1298:1359) at async Object.env (./node_modules/chromaui/addon-visual-tests/dist/index.js:14:290) at async Promise.all (index 3) at async getConfig (./node_modules/storybook/builder-manager/dist/index.js:1:5670) at async Promise.all (index 1) ``` 4. Creating a repo on Github and running `git remote add origin `[`gitgithub.com`](mailto:gitgithub.com)`:zol/test-visual-testing-addon.git` Gets me past the above but now I hit: ``` info => Starting manager.. Unhandled promise rejection: Error: ✖ Unable to execute command: git --no-pager log -n 1 --format="%H ## %ct ## %ae ## %an" Chromatic requires your Git repository to have at least one commit. at Ki (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:287:334) at processTicksAndRejections (node:internal/process/task_queues:96:5) at async cm (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:287:677) at async Object.N3i (/Users/zol/Git/test-visual-testing-addon/node_modules/chromatic/dist/chunk-J3MH667B.js:1298:1385) at async c (/Users/zol/Git/test-visual-testing-addon/node_modules/chromaui/addon-visual-tests/dist/index.js:12:1250) Error: ✖ Unable to execute command: git --no-pager log -n 1 --format="%H ## %ct ## %ae ## %an" Chromatic requires your Git repository to have at least one commit. at Ki (./node_modules/chromatic/dist/chunk-J3MH667B.js:287:334) at processTicksAndRejections (node:internal/process/task_queues:96:5) at async cm (./node_modules/chromatic/dist/chunk-J3MH667B.js:287:677) at async Object.N3i (./node_modules/chromatic/dist/chunk-J3MH667B.js:1298:1385) at async Object.env (./node_modules/chromaui/addon-visual-tests/dist/index.js:14:290) at async Promise.all (index 3) at async getConfig (./node_modules/storybook/builder-manager/dist/index.js:1:5670) at async Promise.all (index 1) at async getData (./node_modules/storybook/builder-manager/dist/index.js:1:4512) at async starter (./node_modules/storybook/builder-manager/dist/index.js:1:7579) WARN Broken build, fix the error above. WARN You may need to refresh the browser. ``` 5. I followed the FAQ item in Chromatic's docs and by taking it's debugging steps I learnt that I needed at least one commit for Chromatic to work.

weeksling commented 11 months ago

@MichaelArestad I added the notification here with the same title and subheader but added a final callout to restart the storybook server. Does that seem alright to you?

Screen Shot 2023-10-03 at 7 42 34 PM
weeksling commented 11 months ago

I wonder if the notification would be weird for first time SB users who haven't even opened the addon yet. Perhaps we should play it cool and wait for them to reach the addon before showing them the message?

I agree. Removing the notification. I also updated it to keep the loop going so they don't have to restart storybook-server. The issue I thought I had with unsetting an addon state variable was an issue in my code

MichaelArestad commented 11 months ago

@MichaelArestad I added the notification here with the same title and subheader but added a final callout to restart the storybook server. Does that seem alright to you?

@weeksling In what scenario would folks see this? Is it necessary?

weeksling commented 11 months ago

@MichaelArestad I added the notification here with the same title and subheader but added a final callout to restart the storybook server. Does that seem alright to you?

@weeksling In what scenario would folks see this? Is it necessary?

No I don't think this is necessary. I removed the notification entirely. Users will see the error when they try to check the addon panel. I thought this was necessary to draw attention to, but it will be happening most often for new storybooks so it's noisy.

MichaelArestad commented 11 months ago

Perfect. Thank you!