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.

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

Send Percy screenshots via npm script (not Jest) #3009

Merged
merged 2 commits into from Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 26 additions & 3 deletions .github/workflows/tests.yml
Expand Up @@ -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"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the CPU load from the @percy/puppeteer Chromium tabs we can now run JavaScript component tests with 2x workers

We use the same flag for JavaScript behaviour tests above


steps:
- name: Checkout
Expand All @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review app uses port 3000 but our Puppeteer helpers want 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 &
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Starts the review app in the background using &


- name: Send screenshots to Percy
run: npx percy exec -- npm run test:screenshots
2 changes: 1 addition & 1 deletion docs/releasing/testing-and-linting.md
Expand Up @@ -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`.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for keeping the docs in sync 🙌🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@36degrees No problem. I didn't quite know if we should keep the test:* prefix

Based on the "…‘tests’ aren’t really tests” feedback

But we can always revisit another day. Thanks again 😊


### 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:
Expand Down
2 changes: 1 addition & 1 deletion lib/puppeteer-helpers.js
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions package.json
Expand Up @@ -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\""
Expand All @@ -44,14 +45,14 @@
"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",
"gulp-rename": "^2.0.0",
"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",
Expand Down Expand Up @@ -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"
}
}
51 changes: 0 additions & 51 deletions src/govuk/components/all.test.js

This file was deleted.

70 changes: 70 additions & 0 deletions tasks/screenshot-components.mjs
@@ -0,0 +1,70 @@
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<void>}
*/
export async function screenshotComponents () {
const browser = await launch(configPuppeteer.launch)
const componentNames = await getDirectories(configPaths.components)

// Screenshot each component
const input = [...componentNames]

// Screenshot 4x components concurrently
Copy link
Member

Choose a reason for hiding this comment

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

How did you arrive at 4x? Might be useful to capture in the commit message in case we want to revisit in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried 5x initially but 4x was faster

Good point though, worth trying 2x and 3x as well (you never know!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you click into the Send screenshots to Percy job to see the timings:

The total action time varies due to GitHub cache retrieval API times being wobbly

I'll keep it at 4x but add a comment

while (input.length) {
const batch = input.splice(0, 4)

await Promise.all(batch
.map(async (componentName) => 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<void>}
*/
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()