From bb2040851d283d2360247cbd10f31e44485d4f3d Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 14 May 2020 13:15:42 +0200 Subject: [PATCH] Take percy snapshots of storybooks (#10622) --- .prettierignore | 1 + .storybook/addons.ts | 3 - .storybook/config.ts | 39 ---- .storybook/coverage.test.ts | 20 +++ .storybook/jest.config.js | 14 ++ .storybook/main.js | 61 +++++++ .storybook/preview.js | 16 ++ .storybook/styles.ts | 15 -- .storybook/webpack.config.ts | 39 ---- dev/codecov.yml | 2 +- enterprise/dev/ci/ci/pipeline-steps.go | 14 +- jest.config.base.js | 2 +- jest.config.js | 1 + package.json | 13 +- .../activation/ActivationDropdown.tsx | 5 +- shared/src/e2e/coverage.ts | 74 ++++---- yarn.lock | 169 +++++++++++++++--- 17 files changed, 321 insertions(+), 167 deletions(-) delete mode 100644 .storybook/addons.ts delete mode 100644 .storybook/config.ts create mode 100644 .storybook/coverage.test.ts create mode 100644 .storybook/jest.config.js create mode 100644 .storybook/main.js create mode 100644 .storybook/preview.js delete mode 100644 .storybook/styles.ts delete mode 100644 .storybook/webpack.config.ts diff --git a/.prettierignore b/.prettierignore index 37f7112213c..29771ced5b7 100644 --- a/.prettierignore +++ b/.prettierignore @@ -26,3 +26,4 @@ shared/dev/**/*.js client/contrib/GH2SG.bookmarklet.js docker-images/grafana/jsonnet/*.json docker-images/grafana/grafonnet-lib/ +!/.storybook/** diff --git a/.storybook/addons.ts b/.storybook/addons.ts deleted file mode 100644 index 41a31c20706..00000000000 --- a/.storybook/addons.ts +++ /dev/null @@ -1,3 +0,0 @@ -import '@storybook/addon-actions/register' -import '@storybook/addon-knobs/register' -import '@storybook/addon-options/register' diff --git a/.storybook/config.ts b/.storybook/config.ts deleted file mode 100644 index 7ef7bdd7413..00000000000 --- a/.storybook/config.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { configureActions } from '@storybook/addon-actions' -// @ts-ignore -import { withConsole } from '@storybook/addon-console' -import { withInfo } from '@storybook/addon-info' -import { withKnobs } from '@storybook/addon-knobs' -import { addDecorator, addParameters, configure } from '@storybook/react' -import { themes } from '@storybook/theming' -import { setLinkComponent, AnchorLink } from '../shared/src/components/Link' - -setLinkComponent(AnchorLink) - -async function main(): Promise { - // Webpack provides require.context. TODO: If this is run in Jest in the future, we'll need to - // use babel-plugin-require-context-hook. - const requireContexts = [ - require.context('../shared', true, /\.story\.tsx?$/), - require.context('../browser', true, /\.story\.tsx?$/), - require.context('../web', true, /\.story\.tsx?$/), - ] - for (const requireContext of requireContexts) { - for (const storyModule of requireContext.keys()) { - requireContext(storyModule) - } - } - - // Configure storybooks. - configure(() => { - addDecorator((storyFn, context) => withKnobs(storyFn, context)) - addDecorator((storyFn, context) => withConsole()(storyFn)(context)) - addParameters({ theme: themes.dark }) - addDecorator(withInfo({ header: false, propTables: false })) - - configureActions({ - depth: 100, - limit: 20, - }) - }, module) -} -main().catch(err => console.error(err)) diff --git a/.storybook/coverage.test.ts b/.storybook/coverage.test.ts new file mode 100644 index 00000000000..d28c9854e74 --- /dev/null +++ b/.storybook/coverage.test.ts @@ -0,0 +1,20 @@ +import initStoryshots from '@storybook/addon-storyshots' +import { puppeteerTest } from '@storybook/addon-storyshots-puppeteer' +import * as path from 'path' +import { pathToFileURL } from 'url' +import { recordCoverage } from '../shared/src/e2e/coverage' + +// This test suite does not actually test anything. +// It just loads up the storybook in Puppeteer and records its coverage, +// so it can be tracked in Codecov. + +initStoryshots({ + configPath: __dirname, + suite: 'Storybook', + test: puppeteerTest({ + storybookUrl: pathToFileURL(path.resolve(__dirname, '../storybook-static')).href, + testBody: async page => { + await recordCoverage(page.browser()) + }, + }), +}) diff --git a/.storybook/jest.config.js b/.storybook/jest.config.js new file mode 100644 index 00000000000..51edb1bf556 --- /dev/null +++ b/.storybook/jest.config.js @@ -0,0 +1,14 @@ +// @ts-check + +const config = require('../jest.config.base') + +const exportedConfig = { + ...config, + displayName: 'storybooks', + rootDir: __dirname, + collectCoverage: false, // Collected through Puppeteer + roots: [''], + verbose: true, +} + +module.exports = exportedConfig diff --git a/.storybook/main.js b/.storybook/main.js new file mode 100644 index 00000000000..9c4cefa0a55 --- /dev/null +++ b/.storybook/main.js @@ -0,0 +1,61 @@ +const path = require('path') +const { remove } = require('lodash') +const { DefinePlugin, ProgressPlugin } = require('webpack') + +const config = { + stories: ['../**/*.story.tsx'], + addons: ['@storybook/addon-knobs', '@storybook/addon-actions', '@storybook/addon-options'], + /** + * @param config {import('webpack').Configuration} + * @returns {import('webpack').Configuration} + */ + webpackFinal: config => { + // Include sourcemaps + config.mode = 'development' + const definePlugin = config.plugins.find(plugin => plugin instanceof DefinePlugin) + // @ts-ignore + definePlugin.definitions.NODE_ENV = JSON.stringify('development') + // @ts-ignore + definePlugin.definitions['process.env'].NODE_ENV = JSON.stringify('development') + + // We don't use Storybook's default config for our repo, it doesn't handle TypeScript. + config.module.rules.splice(0, 1) + + if (process.env.CI) { + remove(config.plugins, plugin => plugin instanceof ProgressPlugin) + } + + config.module.rules.push({ + test: /\.tsx?$/, + loader: require.resolve('babel-loader'), + options: { + configFile: path.resolve(__dirname, '..', 'babel.config.js'), + }, + }) + + config.resolve.extensions.push('.ts', '.tsx') + + // Put our style rules at the beginning so they're processed by the time it + // gets to storybook's style rules. + config.module.rules.unshift({ + test: /\.(css|sass|scss)$/, + use: [ + 'to-string-loader', + 'css-loader', + { + loader: 'sass-loader', + options: { + sassOptions: { + includePaths: [path.resolve(__dirname, '..', 'node_modules')], + }, + }, + }, + ], + // Make sure Storybook styles get handled by the Storybook config + exclude: /node_modules\/@storybook\//, + }) + + return config + }, +} +module.exports = config diff --git a/.storybook/preview.js b/.storybook/preview.js new file mode 100644 index 00000000000..9ff303b9c10 --- /dev/null +++ b/.storybook/preview.js @@ -0,0 +1,16 @@ +import { configureActions } from '@storybook/addon-actions' +import { withConsole } from '@storybook/addon-console' +import { withInfo } from '@storybook/addon-info' +import { withKnobs } from '@storybook/addon-knobs' +import { addDecorator } from '@storybook/react' +import { setLinkComponent, AnchorLink } from '../shared/src/components/Link' + +setLinkComponent(AnchorLink) + +// Don't know why this type doesn't work, but this is the correct usage +// @ts-ignore +addDecorator(withInfo({ header: false, propTables: false })) +addDecorator(withKnobs) +addDecorator((storyFn, context) => withConsole()(storyFn)(context)) + +configureActions({ depth: 100, limit: 20 }) diff --git a/.storybook/styles.ts b/.storybook/styles.ts deleted file mode 100644 index 6adbb9b5b0b..00000000000 --- a/.storybook/styles.ts +++ /dev/null @@ -1,15 +0,0 @@ -// Set commonly used CSS variables that many components assume exist. This is better than importing -// all of (e.g.) SourcegraphWebApp.scss, because those styles are only applied for the web app and -// would be misleading to use for browser extension and shared component storybooks. -if (document && document.documentElement) { - // It's not necessary to define all CSS variables here or to use the precise values from our CSS. - // These are just used for storybooks. - const CSS_VARS = { - '--secondary': '#777777', - '--text-muted': '#bbbbbb', - '--primary': '#1c7ed6', - } - for (const name of Object.keys(CSS_VARS)) { - document.documentElement.style.setProperty(name, CSS_VARS[name]) - } -} diff --git a/.storybook/webpack.config.ts b/.storybook/webpack.config.ts deleted file mode 100644 index 64009e5cafe..00000000000 --- a/.storybook/webpack.config.ts +++ /dev/null @@ -1,39 +0,0 @@ -import * as path from 'path' -import * as webpack from 'webpack' - -export default ({ config }: { config: webpack.Configuration }) => { - if (!config.module || !config.resolve?.extensions) { - throw new Error('unexpected config') - } - - config.module.rules.push({ - test: /\.tsx?$/, - loader: require.resolve('babel-loader'), - options: { - configFile: path.resolve(__dirname, '..', 'babel.config.js'), - }, - }) - config.resolve.extensions.push('.ts', '.tsx') - - // Put our style rules at the beginning so they're processed by the time it - // gets to storybook's style rules. - config.module.rules.unshift({ - test: /\.(css|sass|scss)$/, - use: [ - 'to-string-loader', - 'css-loader', - { - loader: 'sass-loader', - options: { - sassOptions: { - includePaths: [path.resolve(__dirname, '..', 'node_modules')], - }, - }, - }, - ], - // Make sure Storybook styles get handled by the Storybook config - exclude: /node_modules\/@storybook\//, - }) - - return config -} diff --git a/dev/codecov.yml b/dev/codecov.yml index 1f65df961a2..0812e80d5fa 100644 --- a/dev/codecov.yml +++ b/dev/codecov.yml @@ -29,6 +29,6 @@ comment: # Exceptions: # - PRs that only update docs, we don't need Codecov there. # - e2e tests, which don't run on most PRs. - after_n_builds: 5 + after_n_builds: 6 ignore: - '**/bindata.go' diff --git a/enterprise/dev/ci/ci/pipeline-steps.go b/enterprise/dev/ci/ci/pipeline-steps.go index 100f3ce4951..95170446dcc 100644 --- a/enterprise/dev/ci/ci/pipeline-steps.go +++ b/enterprise/dev/ci/ci/pipeline-steps.go @@ -107,8 +107,18 @@ func addSharedTests(pipeline *bk.Pipeline) { bk.Cmd("dev/ci/yarn-test.sh shared"), bk.Cmd("bash <(curl -s https://codecov.io/bash) -c -F typescript -F unit")) - // Storybook - pipeline.AddStep(":storybook:", bk.Cmd("dev/ci/yarn-run.sh storybook:smoke-test")) + // Storybook coverage + pipeline.AddStep(":storybook::codecov:", + bk.Env("PUPPETEER_SKIP_CHROMIUM_DOWNLOAD", ""), + bk.Cmd("COVERAGE_INSTRUMENT=true dev/ci/yarn-run.sh build-storybook"), + bk.Cmd("yarn run cover-storybook"), + bk.Cmd("yarn nyc report -r json"), + bk.Cmd("bash <(curl -s https://codecov.io/bash) -c -F typescript -F storybook")) + + // Upload storybook to Percy + pipeline.AddStep(":storybook::percy:", + bk.Env("PUPPETEER_SKIP_CHROMIUM_DOWNLOAD", ""), + bk.Cmd("dev/ci/yarn-run.sh build-storybook percy-storybook")) } // Adds PostgreSQL backcompat tests. diff --git a/jest.config.base.js b/jest.config.base.js index 16c4be60d74..a10dab611c6 100644 --- a/jest.config.base.js +++ b/jest.config.base.js @@ -11,7 +11,7 @@ const config = { collectCoverage: !!process.env.CI, collectCoverageFrom: ['/src/**/*.{ts,tsx}'], coverageDirectory: '/coverage', - coveragePathIgnorePatterns: [/\.(test|story)\.tsx?$/.source, /\.d\.ts$/.source], + coveragePathIgnorePatterns: [/\/node_modules\//.source, /\.(test|story)\.tsx?$/.source, /\.d\.ts$/.source], roots: ['/src'], // Transform packages that do not distribute CommonJS packages (typically because they only distribute ES6 diff --git a/jest.config.js b/jest.config.js index 035d98059ac..74aeaaa5938 100644 --- a/jest.config.js +++ b/jest.config.js @@ -10,5 +10,6 @@ module.exports = { 'shared/jest.config.js', 'web/jest.config.js', 'cmd/precise-code-intel/jest.config.js', + '.storybook/jest.config.js', ], } diff --git a/package.json b/package.json index fb7855c1521..d98ac1e69bb 100644 --- a/package.json +++ b/package.json @@ -20,12 +20,13 @@ "graphql": "gulp graphQLTypes", "graphql-lint": "graphql-schema-linter --old-implements-syntax --comment-descriptions cmd/frontend/graphqlbackend/schema.graphql", "prepublish": "gulp generate", - "test": "jest --testPathIgnorePatterns e2e regression integration", + "test": "jest --testPathIgnorePatterns e2e regression integration storybook", "test-e2e": "TS_NODE_PROJECT=web/src/e2e/tsconfig.json mocha ./web/src/e2e/e2e.test.ts", "cover-e2e": "nyc --hook-require=false yarn test-e2e", "storybook": "start-storybook -p 9001 -c .storybook", - "storybook:build": "build-storybook -c .storybook", - "storybook:smoke-test": "yarn run storybook --smoke-test", + "build-storybook": "build-storybook -c .storybook", + "cover-storybook": "nyc --hook-require=false yarn jest .storybook/coverage", + "percy-storybook": "PERCY_TOKEN=${PERCY_STORYBOOK_TOKEN} percy-storybook --widths=768", "deduplicate": "yarn-deduplicate -s fewer" }, "browserslist": [ @@ -37,13 +38,14 @@ "not IE > 0" ], "nyc": { + "extends": "@istanbuljs/nyc-config-typescript", "all": true, "extension": [ ".tsx", ".ts" ], "include": [ - "@(web|shared)/src/**/*.ts?(x)" + "@(web|shared|browser)/src/**/*.ts?(x)" ], "exclude": [ "node_modules", @@ -72,8 +74,10 @@ "@babel/runtime": "^7.9.6", "@gql2ts/from-schema": "^1.10.1", "@gql2ts/language-typescript": "^1.9.0", + "@istanbuljs/nyc-config-typescript": "^1.0.1", "@octokit/rest": "^16.36.0", "@percy/puppeteer": "^1.1.0", + "@percy/storybook": "^3.3.0", "@slack/web-api": "^5.8.0", "@sourcegraph/babel-plugin-transform-react-hot-loader-wrapper": "^1.0.0", "@sourcegraph/eslint-config": "^0.17.2", @@ -87,6 +91,7 @@ "@storybook/addon-links": "^5.3.18", "@storybook/addon-options": "^5.3.18", "@storybook/addon-storyshots": "^5.3.18", + "@storybook/addon-storyshots-puppeteer": "^5.3.18", "@storybook/addons": "^5.3.18", "@storybook/components": "^5.3.18", "@storybook/core": "^5.3.18", diff --git a/shared/src/components/activation/ActivationDropdown.tsx b/shared/src/components/activation/ActivationDropdown.tsx index 70f8b9fde8b..f11d561d931 100644 --- a/shared/src/components/activation/ActivationDropdown.tsx +++ b/shared/src/components/activation/ActivationDropdown.tsx @@ -122,7 +122,10 @@ export class ActivationDropdown extends React.PureComponent