From fd46daa033b081d1e9da73431a89249d4e1a9374 Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Fri, 18 Nov 2022 12:03:25 +0000 Subject: [PATCH 1/2] Send Percy screenshots via npm script --- .github/workflows/tests.yml | 29 ++++++++++-- docs/releasing/testing-and-linting.md | 2 +- lib/puppeteer-helpers.js | 2 +- package-lock.json | 3 +- package.json | 6 +-- src/govuk/components/all.test.js | 51 ---------------------- tasks/screenshot-components.mjs | 63 +++++++++++++++++++++++++++ 7 files changed, 95 insertions(+), 61 deletions(-) delete mode 100644 src/govuk/components/all.test.js create mode 100644 tasks/screenshot-components.mjs diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 017cbbbbb9..1940fb8ef2 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -140,7 +140,7 @@ jobs: - description: JavaScript component tests name: test-component - run: npx percy exec -- jest --color --selectProjects "JavaScript component tests" + run: npx jest --color --maxWorkers=2 --selectProjects "JavaScript component tests" steps: - name: Checkout @@ -160,5 +160,28 @@ jobs: - name: Run verify task run: ${{ matrix.run }} - env: - PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }} + + regression: + name: Send screenshots to Percy + runs-on: ubuntu-latest + needs: [install, build] + + env: + PERCY_TOKEN: ${{ secrets.PERCY_TOKEN }} + PORT: 8888 + + steps: + - name: Checkout + uses: actions/checkout@v3 + + - name: Restore dependencies + uses: ./.github/workflows/actions/install-node + + - name: Restore build + uses: ./.github/workflows/actions/build + + - name: Start review application + run: npm run serve & + + - name: Send screenshots to Percy + run: npx percy exec -- npm run test:screenshots diff --git a/docs/releasing/testing-and-linting.md b/docs/releasing/testing-and-linting.md index 67c73399c7..4c71989ec7 100644 --- a/docs/releasing/testing-and-linting.md +++ b/docs/releasing/testing-and-linting.md @@ -101,7 +101,7 @@ We generate 2 screenshots for each default example of every component. One examp The screenshots are public, so you can check them without logging in. A BrowserStack account is needed to approve or reject any changes (if you don't have access, ask your tech lead for help). If you're the reviewer of the pull request code, it's your responsibility to approve or request changes for any visual changes Percy highlights. -When you run the tests locally (for example, using `npm test`), Percy commands are ignored and Percy does not generate any screenshots. You will see the following message in your command line output: `[percy] Percy is not running, disabling snapshots`. +When you run the tests locally (for example, using `npm run test:screenshots`), Percy commands are ignored and Percy does not generate any screenshots. You will see the following message in your command line output: `[percy] Percy is not running, disabling snapshots`. ### PRs from forks When Github Actions is running against a PR from a fork, the Percy secret is not available and Percy does not generate any screenshots. Other tests will continue to run as normal. You will see the following messages in the output: diff --git a/lib/puppeteer-helpers.js b/lib/puppeteer-helpers.js index 751736e8b8..ee0d0b3bdb 100644 --- a/lib/puppeteer-helpers.js +++ b/lib/puppeteer-helpers.js @@ -2,7 +2,7 @@ const { componentNameToJavaScriptClassName } = require('./helper-functions.js') const { renderHtml } = require('./jest-helpers.js') const configPaths = require('../config/paths.js') -const PORT = configPaths.ports.test +const PORT = process.env.PORT || configPaths.ports.test /** * Render and initialise a component within test boilerplate HTML diff --git a/package-lock.json b/package-lock.json index 218bcf40ea..a07e7bebd2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -74,8 +74,7 @@ "stylelint": "^14.15.0", "stylelint-config-gds": "^0.2.0", "stylelint-order": "^5.0.0", - "wait-on": "^6.0.1", - "ws": "^8.11.0" + "wait-on": "^6.0.1" }, "engines": { "node": "^16.13.0", diff --git a/package.json b/package.json index b29a8bed61..3a6b52f136 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "postbuild:dist": "jest --color --selectProjects \"Gulp tasks\" --testMatch \"**/*build-dist*\"", "pretest": "npm run build:compile", "test": "jest --color --ignoreProjects \"Gulp tasks\" --maxWorkers=50%", + "test:screenshots": "node ./tasks/screenshot-components.mjs", "lint": "npm run lint:js && npm run lint:scss", "lint:js": "eslint --cache --cache-location .cache/eslint --cache-strategy content --color --ignore-path .gitignore --max-warnings 0 \"**/*.{cjs,js,mjs}\"", "lint:scss": "stylelint --cache --cache-location .cache/stylelint --cache-strategy content --color --ignore-path .gitignore --max-warnings 0 \"{app,src}/**/*.scss\"" @@ -44,6 +45,7 @@ "glob": "^8.0.3", "gulp": "^4.0.2", "gulp-better-rollup": "3.1.0", + "gulp-cli": "^2.3.0", "gulp-if": "^3.0.0", "gulp-plumber": "^1.2.1", "gulp-postcss": "^9.0.1", @@ -51,7 +53,6 @@ "gulp-sass": "^5.1.0", "gulp-task-listing": "^1.1.1", "gulp-uglify": "^3.0.2", - "gulp-cli": "^2.3.0", "js-yaml": "^4.1.0", "jsdoc": "^4.0.0", "jsdoc-tsimport-plugin": "^1.0.5", @@ -95,7 +96,6 @@ "stylelint": "^14.15.0", "stylelint-config-gds": "^0.2.0", "stylelint-order": "^5.0.0", - "wait-on": "^6.0.1", - "ws": "^8.11.0" + "wait-on": "^6.0.1" } } diff --git a/src/govuk/components/all.test.js b/src/govuk/components/all.test.js deleted file mode 100644 index 69db3391d4..0000000000 --- a/src/govuk/components/all.test.js +++ /dev/null @@ -1,51 +0,0 @@ -const { join } = require('path') -const { fetch } = require('undici') -const { WebSocket } = require('ws') - -const { getDirectories, getListing } = require('../../../lib/file-helper') -const { goToComponent } = require('../../../lib/puppeteer-helpers') - -const configPaths = require('../../../config/paths.js') - -describe('Visual regression via Percy', () => { - let percySnapshot - - let componentsFiles - let componentNames - - beforeAll(async () => { - // Polyfill fetch() detection, upload via WebSocket() - // Fixes Percy running in a non-browser environment - global.window = { fetch, WebSocket } - percySnapshot = require('@percy/puppeteer') - - // Component directory listing (with contents) - componentsFiles = await getListing(configPaths.components) - - // Components list - componentNames = await getDirectories(configPaths.components) - }) - - afterAll(async () => { - await page.setJavaScriptEnabled(true) - }) - - it('generate screenshots', async () => { - for (const componentName of componentNames) { - await page.setJavaScriptEnabled(true) - - // Screenshot preview page (with JavaScript) - await goToComponent(page, componentName) - await percySnapshot(page, `js: ${componentName}`) - - // Check for "JavaScript enabled" components - if (componentsFiles.includes(join(componentName, `${componentName}.mjs`))) { - await page.setJavaScriptEnabled(false) - - // Screenshot preview page (without JavaScript) - await page.reload({ waitUntil: 'load' }) - await percySnapshot(page, `no-js: ${componentName}`) - } - } - }, 120000) -}) diff --git a/tasks/screenshot-components.mjs b/tasks/screenshot-components.mjs new file mode 100644 index 0000000000..d274d0590c --- /dev/null +++ b/tasks/screenshot-components.mjs @@ -0,0 +1,63 @@ +import { join } from 'path' +import { launch } from 'puppeteer' +import percySnapshot from '@percy/puppeteer' +import { isPercyEnabled } from '@percy/sdk-utils' + +import { getDirectories, getListing } from '../lib/file-helper.js' +import { goToComponent } from '../lib/puppeteer-helpers.js' + +import configPaths from '../config/paths.js' +import configPuppeteer from '../puppeteer.config.js' + +/** + * Send all component screenshots to Percy + * for visual regression testing + * + * @returns {Promise} + */ +export async function screenshotComponents () { + const browser = await launch(configPuppeteer.launch) + const componentNames = await getDirectories(configPaths.components) + + // Screenshot each component in sequence + for (const componentName of componentNames) { + await screenshotComponent(await browser.newPage(), componentName) + } + + // Close browser + return browser.close() +} + +/** + * Send single component screenshots to Percy + * for visual regression testing + * + * @param {import('puppeteer').Page} page - Puppeteer page object + * @param {string} componentName - Component name + * @returns {Promise} + */ +export async function screenshotComponent (page, componentName) { + const componentFiles = await getListing(configPaths.components, `**/${componentName}/**`) + + // Screenshot preview page (with JavaScript) + await goToComponent(page, componentName) + await percySnapshot(page, `js: ${componentName}`) + + // Check for "JavaScript enabled" components + if (componentFiles.includes(join(componentName, `${componentName}.mjs`))) { + await page.setJavaScriptEnabled(false) + + // Screenshot preview page (without JavaScript) + await page.reload({ waitUntil: 'load' }) + await percySnapshot(page, `no-js: ${componentName}`) + } + + // Close page + return page.close() +} + +if (!await isPercyEnabled()) { + throw new Error('Percy healthcheck failed') +} + +await screenshotComponents() From 93cde6f7cf11450ea1cb635dc1fe99915b13939d Mon Sep 17 00:00:00 2001 From: Colin Rotherham Date: Fri, 18 Nov 2022 11:20:08 +0000 Subject: [PATCH 2/2] Send Percy screenshots in batches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs faster than “one at a time” by using multiple Chromium tabs. Once we go above 4x though we see little extra benefit See: https://github.com/alphagov/govuk-frontend/pull/3009#discussion_r1026336842 --- tasks/screenshot-components.mjs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tasks/screenshot-components.mjs b/tasks/screenshot-components.mjs index d274d0590c..519dbc8521 100644 --- a/tasks/screenshot-components.mjs +++ b/tasks/screenshot-components.mjs @@ -19,9 +19,16 @@ export async function screenshotComponents () { const browser = await launch(configPuppeteer.launch) const componentNames = await getDirectories(configPaths.components) - // Screenshot each component in sequence - for (const componentName of componentNames) { - await screenshotComponent(await browser.newPage(), componentName) + // Screenshot each component + const input = [...componentNames] + + // Screenshot 4x components concurrently + while (input.length) { + const batch = input.splice(0, 4) + + await Promise.all(batch + .map(async (componentName) => screenshotComponent(await browser.newPage(), componentName)) + ) } // Close browser