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.

Sign up for GitHub

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

Jump to bottom

Add storybook ci option to test that Storybook starts "ok" #3515

Merged
merged 14 commits into from Jan 24, 2022
Merged

Add storybook ci option to test that Storybook starts "ok" #3515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
83b0452
add rw storybook --ci option passing --smoke-test
thedavidprice Jul 28, 2021
daec4ea
fix: do not stub `process.env`
virtuoushub Dec 23, 2021
ef20e2b
chore: enable typechecking
virtuoushub Jan 12, 2022
8be9491
refactor(rename): ci -> smoke-test
virtuoushub Jan 12, 2022
8afd1c8
chore: add some logic checks
virtuoushub Jan 12, 2022
f899152
feat: wire up smoke test
virtuoushub Jan 12, 2022
c04a676
fix: run checker in synchronously mode
virtuoushub Jan 12, 2022
d060185
chore: shorten timeout and fail faster
virtuoushub Jan 13, 2022
683745a
chore: try and move this into a before
virtuoushub Jan 13, 2022
a17b8e0
use sb --smoke-test for E2E; remove Cypress spec
thedavidprice Jan 13, 2022
e71dd9d
fix: only set up type checking for typescript projects
thedavidprice Jan 13, 2022
1d42a01
Merge branch 'main' into chore/add-storybook-ci-option-to-test-start-…
thedavidprice Jan 22, 2022
c7a6b85
Merge branch 'main' into chore/add-storybook-ci-option-to-test-start-…
thedavidprice Jan 24, 2022
6014b79
Update tasks/e2e/cypress/integration/03-storybook/storybook.spec.js
thedavidprice Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/e2e.yaml
View file
Open in desktop
Expand Up @@ -82,14 +82,17 @@ jobs:
working-directory: ./tasks/e2e
spec: |
cypress/integration/01-tutorial/*.spec.js
cypress/integration/03-storybook/*.spec.js
cypress/integration/04-logger/*.spec.js

- name: Prepare for CLI checks by restoring 01-tutorial end state
run: |
git restore . && git clean -df
working-directory: ${{ steps.createpath.outputs.project_path }}

- name: Run `rw storybook`
run: yarn rw storybook --smoke-test
working-directory: ${{ steps.createpath.outputs.project_path }}

- name: Run `rw test api`
run: yarn rw test api --no-watch
working-directory: ${{ steps.createpath.outputs.project_path }}
Expand Down
19 changes: 18 additions & 1 deletion packages/cli/src/commands/storybook.js
View file
Open in desktop
Expand Up @@ -37,6 +37,21 @@ export const builder = (yargs) => {
type: 'boolean',
default: true,
})
.option('smoke-test', {
describe:
"CI mode plus Smoke-test (skip prompts, don't open browser, exit after successful start)",
type: 'boolean',
default: false,
})
.check((argv) => {
if (argv.build && argv.smokeTest) {
throw new Error('Can not provide both "--build" and "--smoke-test"')
}
if (argv.build && argv.open) {
throw new Error('Can not provide both "--build" or "--open"')
}
return true
})
}

export const handler = ({
Expand All @@ -45,6 +60,7 @@ export const handler = ({
build,
buildDirectory,
managerCache,
smokeTest,
}) => {
const cwd = getPaths().web.base

Expand All @@ -69,7 +85,8 @@ export const handler = ({
!build && '--no-version-updates',
!managerCache && '--no-manager-cache',
build && `--output-dir "${buildDirectory}"`,
!open && '--ci',
!open && !smokeTest && `--ci`,
smokeTest && `--ci --smoke-test`,
].filter(Boolean),
{
stdio: 'inherit',
Expand Down
1 change: 1 addition & 0 deletions packages/core/config/webpack.common.js
View file
Open in desktop
Expand Up @@ -172,6 +172,7 @@ const getSharedPlugins = (isEnvProduction) => {
new Dotenv({
path: path.resolve(redwoodPaths.base, '.env'),
silent: true,
ignoreStub: true, // FIXME: this might not be necessary once the storybook webpack 4/5 stuff is ironed out. See also: https://github.com/mrsteele/dotenv-webpack#processenv-stubbing--replacing
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @virtuoushub, me again! We have a small regression caused by this change here. Wondering if you could give me a little more context to what this change was for/why?

Copy link
Contributor

Choose a reason for hiding this comment

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

Without this, the Storbook --ci and --smoke-test flags were failing. (Please correct me if I am wrong.)

FYI, Pete, Danny is going to remove Dotenv entirely over here #4334 Based on all the research you did, any reaction as to whether this removal will have downstream affect on Storybook config? (We are assuming none.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @dac09 , it was added to avoid:

...
WARN preview: [
WARN   {
WARN     "message": "DefinePlugin\nConflicting values for 'process.env'",
WARN     "details": "'{NODE_ENV: \"development\", NODE_PATH: [], STORYBOOK: \"true\", PUBLIC_URL: \".\"}' !== '\"MISSING_ENV_VAR\"'",
WARN     "stack": "Error: DefinePlugin\nConflicting values for 'process.env'\n    at /workspace/rw-test-app/node_modules/webpack/lib/DefinePlugin.js:573:24\n    at Array.forEach (<anonymous>)\n    at walkDefinitionsForValues (/workspace/rw-test-app/node_modules/webpack/lib/DefinePlugin.js:564:31)\n    at /workspace/rw-test-app/node_modules/webpack/lib/DefinePlugin.js:591:5\n    at Hook.eval [as call] (eval at create (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:291:1)\n    at Hook.CALL_DELEGATE [as _call] (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/Hook.js:14:14)\n    at Compiler.newCompilation (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/lib/Compiler.js:1053:26)\n    at /workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/lib/Compiler.js:1097:29\n    at Hook.eval [as callAsync] (eval at create (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:40:1)\n    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/Hook.js:18:14)\n    at Compiler.compile (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/lib/Compiler.js:1092:28)\n    at /workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/lib/Watching.js:200:19\n    at _next0 (eval at create (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:27:1)\n    at eval (eval at create (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:40:1)\n    at watchRunHook (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack-virtual-modules/lib/index.js:236:13)\n    at Hook.eval [as callAsync] (eval at create (/workspace/rw-test-app/node_modules/@storybook/builder-webpack5/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:34:1)"
WARN   }
WARN ]
...

See also: #3515 (comment)

@thedavidprice , I think the removal should be safe; 🤞🏻 that the smoke test catches a regression if I am wrong.

}),
].filter(Boolean)
}
Expand Down
10 changes: 10 additions & 0 deletions packages/testing/config/storybook/main.js
View file
Open in desktop
Expand Up @@ -9,6 +9,7 @@ const {
getConfig,
getPaths,
} = require('@redwoodjs/internal')
const { getProject } = require('@redwoodjs/structure')

const config = getConfig()

Expand Down Expand Up @@ -136,6 +137,15 @@ const baseConfig = {
...(process.env.NODE_ENV !== 'production' && {
staticDirs: [`${staticAssetsFolder}`],
}),
// only set up type checking for typescript projects
...(getProject().isTypeScriptProject && {
// https://storybook.js.org/docs/react/configure/typescript#mainjs-configuration
typescript: {
check: true,
// By default, the checker runs asynchronously in dev mode. Force it to run synchronously.
checkOptions: { async: false },
},
}),
}

const mergeUserStorybookConfig = (baseConfig) => {
Expand Down
11 changes: 5 additions & 6 deletions tasks/e2e/cypress/integration/03-storybook/storybook.spec.js
View file
Open in desktop
Expand Up @@ -12,7 +12,8 @@ describe(
'Redwood Storybook Integration',
{ baseUrl: 'http://localhost:8910' },
() => {
it('0. Build Storybook Static Files', () => {
// 0. Build Storybook Static Files
before(() => {
cy.writeFile(
path.join(BASE_DIR, 'web/src/components/BlogPost/BlogPost.stories.js'),
Step1_1_BlogPostStory
Expand All @@ -33,13 +34,11 @@ describe(
)

// Slow!
cy.exec(`cd ${BASE_DIR}; yarn rw storybook --build`, {
cy.exec(`cd ${BASE_DIR} || exit; yarn rw storybook --build`, {
timeout: 300_0000,
}).then((result) => {
const { code, stderr } = result
expect(code).to.eq(0)
expect(stderr).to.not.contain('ERR!')
})
.its('stderr')
.should('not.contain', 'ERR!')
})

it('1. Component: BlogPost', () => {
Expand Down