fix: check no "ERR!" string is in stderr when running Storybook build

[virtuoushub]

Avoid false positive. It seems stderr contains info as well as error, so just checking if it is '' will not work.

See Also:

• test(e2e): add stderr check to Storybook build #3508
• https://github.com/redwoodjs/redwood/runs/3799459178?check_suite_focus=true

cc: @thedavidprice

---

note: while #3154 looks to fix the "connection time out" error we were seeing in CI checks; speaking from experience, there still can be inadvertent regressions added to the CI when upgrading Storybook, or other "unrelated" dependencies. This PR ( and #3508 even though the check was wrong ) makes sure that if there are build errors output, the stderr is surfaced to the CI so that maintainers can more easily see the potential issue.

[thedavidprice]

Ok, I'm following you here and the Cypress changes are really helpful to see (I personally am not very experienced with Cypress outside this project).Bare with me as I walk through a thought process that got us here.

This is all just fwiw exploration. I'm game to punt on further exploration and just merge this as is.

False positives and false negatives == why even try 🤷‍♂️

Admittedly this is a hacky, brittle way to test Storybook. It's actually my hack, inspired by the facts 1) we don't have a way to both start and stop a Storybook process using Cypress in this manner (also, resource constraints) and 2) we already have yarn rw dev running which is serving static assets in web/public. Viola... Hack!

So we run into scenarios like this one, where the Storybook spec is failing, but yet it's not really failing. When I run yarn rw sb --build locally I get warnings but no errors:

SB --build locally

info @storybook/react v6.3.8
info 
info => Cleaning outputDir: /Users/price/Repos/tdp-redwood-tutorial-test/web/public/storybook
info => Loading presets
info => Compiling manager..
info => Compiling preview..
info => Loading custom babel config as JS
info => Loading custom babel config as JS
info => Loading 1 config file in "/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/testing/config/storybook"
info => Loading 2 other files in "/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/testing/config/storybook"
info => Adding stories defined in "/Users/price/Repos/tdp-redwood-tutorial-test/node_modules/@redwoodjs/testing/config/storybook/main.js"
info => Using implicit CSS loaders
info => Using default Webpack5 setup
92% sealing asset processing TerserPlugininfo => Manager built (12 s)
info => Preview built (21 s)
WARN DefinePlugin
WARN Conflicting values for 'process.env'
WARN asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
WARN This can impact web performance.
WARN Assets: 
WARN   main.ff1be6a9.iframe.bundle.js (1.86 MiB)
WARN entrypoint size limit: The following entrypoint(s) combined asset size exceeds the recommended limit (244 KiB). This can impact web performance.
WARN Entrypoints:
WARN   main (1.86 MiB)
WARN       static/css/main.62eb66b7.css
WARN       main.ff1be6a9.iframe.bundle.js
WARN 
info => Output directory: /Users/price/Repos/tdp-redwood-tutorial-test/web/public/storybook

And even in the output from #3508 there were no Errors in the stderr:

• E2E CI Run: https://github.com/redwoodjs/redwood/runs/3799459178

Sanity check --> are you seeing something different locally and in the CI runs? Maybe there's a Storybook error buried in there somewhere that needs to be resolved. (But in that case I don't understand why this one is passing...?)

So why was the spec failing?

We've ended up assuming that failures like this are just resource constraints and can't be resolved until we rewrite tests for these cases using something other than Cypress.

@virtuoushub are you seeing something different locally and in the CI runs? It's possible I'm just missing real errors

But maybe what you're doing here is resolving an issue we had with the cy.exec never "returning" anything (error or success), so Cypress was either timing out or waiting or trying to move to the next step before the build process was complete.

I don't know Cypress well enough to know. But given that this is passing, do you think there's actually an improvement beyond error capture?

An alternate test

Originally, we just wanted to check if start-storybook would successfully run the process. This would include the step to build the stories and then fire up the Storybook server.

At one time I looked into using yarn start-storybook --smoke-test, which is designed for this use case. See my draft PR here:

• add storybook ci option to test start-storybook ok #3141

I didn't finish it because I never worked through how to "capture" the output to check in Cypress.

I'll defer to you, but would you be interested in exploring a full replacement of this current Storybook spec with a way to use --smoke-test? (In which case, we wouldn't worry about doing E2E on the individual stories... that's not necessary.)

Note: you can try this locally by passing the path to the Storybook config you see when you run yarn rw sb. E.g:

yarn build-storybook --config-dir path-to-project/node_modules/@redwoodjs/testing/config/storybook --output-dir path-to/output-dir

[virtuoushub]

I'll defer to you, but would you be interested in exploring a full replacement of this current Storybook spec with a way to use --smoke-test? (In which case, we wouldn't worry about doing E2E on the individual stories... that's not necessary.)

Yes, that interests me.

I do advocate in the short term we go with this fix as "hacky" as it is. There is still some value in the checks as they are.

I opened #3511 to illustrate how this change might surface actual build errors.

[thedavidprice]

I do advocate in the short term we go with this fix as "hacky" as it is.

Agreed! Merging now (and letting my original hack continue to propagate).

Will continue discussion in #3511