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

Upgrade browser/accessibility tests for Puppeteer v18 #2532

Merged
merged 9 commits into from
Jan 17, 2023

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Jan 13, 2023

This PR is a fairly drastic bump to puppeteer@1.14.0 puppeteer@18.2.1

Which includes:

  1. Test helpers from govuk-frontend ./lib/puppeteer-helpers.js
  2. Use @axe-core/puppeteer to replace axe-puppeteer
  3. Run Puppeteer (Chromium) in incognito context
  4. Jest + ESLint config additions

Lots of tests have been simplified to the newer Puppeteer selector methods:

await page.$('selector')
await page.$$('selector')

Plus various other anti-flakiness improvements inspired by:

Closes #2468

Switching away from the deprecated `axe-puppeteer` in future unblocks `puppeteer` upgrades:

```
npm ERR! Could not resolve dependency:
npm ERR! peer puppeteer@">=1.10.0 <= 18" from @axe-core/puppeteer@4.5.2
```
Additionally adds `jest-axe` for violation reporting
Recent axe update can’t use `puppeteer@19` yet

```
npm ERR! Could not resolve dependency:
npm ERR! peer puppeteer@">=1.10.0 <= 18" from @axe-core/puppeteer@4.5.2
```
This matches axe behaviour from previous releases, but see discussion: #2442 (comment)
Includes various changes and fixes for the `puppeteer@18` upgrade
@netlify
Copy link

netlify bot commented Jan 13, 2023

You can preview this change here:

Name Link
🔨 Latest commit ea4ec2b
🔍 Latest deploy log https://app.netlify.com/sites/govuk-design-system-preview/deploys/63c1971225455d000819159b
😎 Deploy Preview https://deploy-preview-2532--govuk-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@colinrotherham
Copy link
Contributor Author

Note: We've had to hold off puppeteer@19 due to @axe-core/puppeteer dependency ranges

npm ERR! Could not resolve dependency:
npm ERR! peer puppeteer@">=1.10.0 <= 18" from @axe-core/puppeteer@4.5.2

},
globals: {
page: true,
browser: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allows the jest-puppeteer page and browser globals to work without lint errors

Includes CLI flags for colour output, GitHub Actions reporter, ESLint config changes for test globals
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

Works for me locally and seems speedier?

Got a few comments/queries, but I don't really foresee anything being blocking.

__tests__/accessiblity-audit.test.js Show resolved Hide resolved
__tests__/accessiblity-audit.test.js Show resolved Hide resolved
__tests__/accessiblity-audit.test.js Show resolved Hide resolved
lib/puppeteer-helpers.js Show resolved Hide resolved
jest.setup.js Show resolved Hide resolved
.github/workflows/test.yaml Show resolved Hide resolved
jest.config.mjs Show resolved Hide resolved
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

Good to go!

Good to silence those seemingly daily puppeteer updates...

I'd love to see more thorough accessibility tests, but I think that begins on govuk-frontend, and it's certainly beyond the scope of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Replaceaxe-puppeteer
3 participants