Improvements to Jest project setup + test package updates #2850

colinrotherham · 2022-09-15T08:27:08Z

Lots of little fixes in this PR

Package updates for Jest, Percy, Puppeteer
Fixed Percy window.fetch() (newer versions assume a browser or jsdom environment)
Added caching for Jest rollup transforms (speeds up tests)
Only start Puppeteer (slow) for tests that actually need it
~~Remove deprecated request package~~ Replace deprecated request package with Fetch API #2858

You can now reliably use the Jest CLI to run single tests, filters or target projects:

npx jest --selectProjects 'Snapshot tests'
npx jest --selectProjects 'JavaScript unit tests'
npx jest --selectProjects 'JavaScript behaviour tests'
npx jest --selectProjects 'JavaScript component tests'

☝️ The test site (and puppeteer) will now start/stop automatically when needed

Jest with `--watch` flag

For example, this PR resolves various Jest issues such as:

Error: listen EADDRINUSE: address already in use :::8888
    at Server.setupListenHandle [as _listen2] (node:net:1432:16)
    at listenInCluster (node:net:1480:12)
    at Server.listen (node:net:1568:7)
    at Function.listen (/path/to/project/govuk-frontend/node_modules/express/lib/application.js:618:24)
    at Object.<anonymous> (/path/to/project/govuk-frontend/app/start.js:6:5)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
Emitted 'error' event on Server instance at:
    at emitErrorNT (node:net:1459:8)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  code: 'EADDRINUSE',
  errno: -48,
  syscall: 'listen',
  address: '::',
  port: 8888
}

These can be shown by adding --detectOpenHandles as suggested:

Jest did not exit one second after the test run has completed.

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with `--detectOpenHandles` to troubleshoot this issue.

36degrees

Thanks @colinrotherham!

I can generally understand the changes except for the last commit – I don't really understand what it's fixing or how the changes relate to each other. Would you mind adding a bit more detail to the commit message that explains what's going on, for future us?

36degrees · 2022-09-15T12:33:52Z

jest.config.js

+  cacheDirectory: '<rootDir>/.cache/jest/',
+  testPathIgnorePatterns: [
+    '/node_modules/',
+    'after-*'


I think this has broken the test:build:package and test:build:dist tasks:

govuk-frontend on  upgrade-test-jest-percy [$!] via ⬢ v16.17.0 took 20s ➜ npm run test:build:package > test:build:package > jest tasks/gulp/__tests__/after-build-package.test.js No tests found, exiting with code 1 Run with `--passWithNoTests` to exit with code 0 In /Users/oliver.byford/Code/govuk-frontend 1165 files checked across 4 projects. Run with `--verbose` for more details. Pattern: tasks/gulp/__tests__/after-build-package.test.js - 0 matches govuk-frontend on  upgrade-test-jest-percy [$!] via ⬢ v16.17.0 took 3s ➜ npm run test:build:dist > test:build:dist > jest tasks/gulp/__tests__/after-build-dist.test.js No tests found, exiting with code 1 Run with `--passWithNoTests` to exit with code 0 In /Users/oliver.byford/Code/govuk-frontend 1165 files checked across 4 projects. Run with `--verbose` for more details. Pattern: tasks/gulp/__tests__/after-build-dist.test.js - 0 matches

The desired behaviour is that the tests in tasks/gulp/__tests__/ are excluded from running as part of npm test but we can call them as part of the build:package and build:dist tasks to check that the package / dist are what we expect.

(I suspect there's a better way to do this! Maybe we can use the Jest projects config?)

Don't worry, this is what code reviews are for 😊

I'll find a better way of grouping those excluded-but-not-excluded tests—probably Jest config yep!

@36degrees I've pushed this up again

The GitHub tests are now split out so Percy only monitors what it needs to:
https://github.com/alphagov/govuk-frontend/blob/upgrade-test-jest-percy/.github/workflows/tests.yml#L47

colinrotherham · 2022-09-15T12:53:04Z

src/govuk/components/all.test.js

+
+  beforeAll(() => {
+    // Support fetch() detection, upload via WebSocket()
+    global.window = { fetch, WebSocket }


@36degrees To answer the "Why do we need this?"

For context, Jest used to run with @jest-environment jsdom by default

Performance improvements
https://jestjs.io/blog/2022/04/25/jest-28
https://jestjs.io/blog/2021/05/25/jest-27

Jest now runs all tests via a Node.js environment (no browser globals) but with the recent updates we can't have both @jest-environment jsdom and @jest-environment puppeteer working together

The problem? Percy code is required (by Jest) as a "browserified" bundle, so hits this code path:
https://github.com/percy/cli/blob/19f8be4903543544bdef197f2f7d5a6a648ed37d/packages/sdk-utils/src/logger.js#L74

So this restores both browser globals and fixes:

Fetch API for Percy to see if it's CLI agent is running

WebSocket for Percy to upload screenshots securely

I have been wondering about splitting the Percy snapshotting out as it's not really part of the 'tests' and doesn't rely on anything else from Jest.

I suspect we might also need to do this if we wanted to conditionally run the visual regression tests only if certain files have changed.

Until then, to stop us having to go down that rabbit hole too far, would another way to solve this be to split the snapshotting out into its own Jest test case?

@36degrees Yeah, appreciate this is a quick fix. Did you see the new projects list?

npx jest --selectProjects "JavaScript component tests"

Likewise we can run everything except a certain project:

npx jest --ignoreProjects "Snapshot tests"

Does Percy inherently need both jsdom and puppeteer because we're running it in Jest? Or is this a symptom of the fact we have these tests inside a file with a lot of other ~unrelated tests and some of them need jsdom and some need puppeteer?

No it just needs a browser window object

It's because we use @percy/puppeteer which crashes without window.fetch and window.WebSocket. For performance reasons, Jest CLI no longer defaults to a simulated browser (jsdom) environment

Here's an extract of their code (via browserify bundle) showing .call(window)

(function() { /* Percy internals */ }).call(window); if (typeof define === "function" && define.amd) { define("@percy/sdk-utils", [], () => window.PercySDKUtils); } else if (typeof module === "object" && module.exports) { module.exports = window.PercySDKUtils; }

I'm happy with this fix until we can investigate calling @percy/client directly

This PR is primarily to get jest --watch stable again (starting/stopping test site cleanly etc)

To be honest, looks like a repeat of #2643 again

Jest v28 package.json exports support
…As a result, you might e.g. get browser code which assumes ESM, when Jest provides ['require', 'browser']. You can either report a bug to the library…

Jest now (correctly??) loads the default browser bundle for @percy/puppeteer on this line:
https://github.com/percy/cli/blob/master/packages/sdk-utils/package.json#L27

"exports": { ".": { "node": "./dist/index.js", "default": "./dist/bundle.js" } }

We'll have to stick with this fix and consider a PR with Percy to add import and require exports

colinrotherham · 2022-09-16T08:11:48Z

To help review this one I've split out two of the noisier changes:

#2857 Make config/paths.json exportable for ESM import
#2858 Replace deprecated request package with Fetch API

colinrotherham · 2022-09-16T08:29:59Z

Rebasing with #2857 to pick up config path changes

.github/workflows/tests.yml

36degrees · 2022-09-16T16:08:53Z

.github/workflows/tests.yml

+          npx jest --color --selectProjects "JavaScript unit tests"
+          npx jest --color --selectProjects "JavaScript behaviour tests"
+
+      - name: Run visual regression


This is also running the component tests as well, right? Also not sure if 'Run visual regression' is the right name as the actual regression testing is a separate status check – this just sends the screenshots over to Percy.

Suggested change

- name: Run visual regression

- name: Send screenshots to Percy

Yeah Percy starts a little service on http://localhost:5338/percy/snapshot then runs your command

Our tests call await percySnapshot() which sends the screenshot if Percy started cleanly (with a token)

Thanks @36degrees I've updated this to your suggestion 👍

36degrees · 2022-09-16T16:09:37Z

jest.config.js

+      ]
+    },
+    {
+      ...config,
      displayName: 'Snapshot tests',


Appreciate this name is not new, but it's not what I'd naturally call these. Especially as Jest has its own meaning for 'snapshots'.

Maybe 'Nunjucks macro tests'?

Suggested change

displayName: 'Snapshot tests',

displayName: 'Nunjucks macro tests',

Updated this one too 👍

36degrees · 2022-09-16T16:11:13Z

src/govuk/components/all.test.js

+
+  beforeAll(() => {
+    // Support fetch() detection, upload via WebSocket()
+    global.window = { fetch, WebSocket }


Does Percy inherently need both jsdom and puppeteer because we're running it in Jest? Or is this a symptom of the fact we have these tests inside a file with a lot of other ~unrelated tests and some of them need jsdom and some need puppeteer?

package.json

36degrees · 2022-09-16T16:16:26Z

package.json

-    "test:build:dist": "jest tasks/gulp/__tests__/after-build-dist.test.js",
+    "pretest": "npm run build:assets",
+    "test": "jest --color --ignoreProjects \"Gulp tasks\" --maxWorkers=50%",
+    "test:build:assets": "npx jest --selectProjects \"Gulp tasks\" --testMatch \"**/*components*\"",


Is this intended to run check-individual-components-compile.test.js?

I had no idea that test file existed…

I think it's running on the contents of the src directory, so don't think it makes sense to run it after build:assets as that sort of suggests it's running tests against the copied files in the public directory.

I think the test basically duplicates tests we already have in components/all.test.js:

govuk-frontend/src/govuk/components/all.test.js

Lines 40 to 44 in a1a1944

it(`${component}.scss renders to CSS without errors`, () => {

return renderSass({

file: `${configPaths.src}/components/${component}/_${component}.scss`

})

})

If both of those are correct, I think we should delete check-individual-components-compile.test.js (because it's covered by the tests in src/govuk/components/all.test.js) and remove the test:build:assets script.

Yep that was the intention, agreed. I'll remove 👍

Removed it now, well spotted. Don't need to call renderSass() on every component twice 😊

36degrees · 2022-09-20T07:59:24Z

docs/contributing/tasks.md

@@ -28,6 +28,10 @@ NPM scripts are defined in `package.json`. These trigger a number of gulp tasks.
 - compiles CSS & JS
 - starts up Express

+**`npm run build:assets` will do the following:**
+- compiles CSS & JS
+- runs `npm run test:build:assets` (which will test the output is correct)


This needs removing now that we've gotten rid of test:build:assets

Fixed, thanks @36degrees

Test server (and puppeteer) now starts/stops automatically for only the tests that need it You can now reliably use the Jest CLI to run single tests, filters or target projects: ```js npx jest --selectProjects 'Gulp tasks' npx jest --selectProjects 'Snapshot tests' npx jest --selectProjects 'JavaScript unit tests' npx jest --selectProjects 'JavaScript behaviour tests' npx jest --selectProjects 'JavaScript component tests' ```

Due to missing `require` or `exports` main entry points in Percy SDK utils, Jest is (correctly) loading the browser bundle instead See https://github.com/percy/cli/blob/a9858d20a9b9708da0464c0617b32b2ee1c97433/packages/sdk-utils/package.json#L27

Test was sometimes failing under load, potentially related to emulation mode viewport width not revealing the mobile menu immediately

See ./src/govuk/components/all.test.js

36degrees

Fab work 👏🏻

colinrotherham requested a review from a team as a code owner September 15, 2022 08:27

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 08:27 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from ed92373 to 9889dd9 Compare September 15, 2022 12:14

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 12:14 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 9889dd9 to bf2beda Compare September 15, 2022 12:18

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 12:19 Inactive

36degrees reviewed Sep 15, 2022

View reviewed changes

colinrotherham commented Sep 15, 2022

View reviewed changes

colinrotherham force-pushed the upgrade-test-jest-percy branch from bf2beda to d79e7bd Compare September 15, 2022 15:45

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 15:45 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from d79e7bd to 332367c Compare September 15, 2022 15:49

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 15:50 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 332367c to 0c82db1 Compare September 15, 2022 15:53

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 15:53 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 0c82db1 to 1d91abd Compare September 15, 2022 17:20

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 17:20 Inactive

colinrotherham added this to Needs review 🔍 in Design System Sprint Board Sep 15, 2022

colinrotherham force-pushed the upgrade-test-jest-percy branch from 1d91abd to 70d7d34 Compare September 15, 2022 17:30

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 15, 2022 17:30 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 70d7d34 to d829a3d Compare September 16, 2022 07:48

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 16, 2022 07:48 Inactive

colinrotherham mentioned this pull request Sep 16, 2022

Replace deprecated request package with Fetch API #2858

Merged

colinrotherham added tooling javascript squad labels Sep 16, 2022

colinrotherham self-assigned this Sep 16, 2022

colinrotherham force-pushed the upgrade-test-jest-percy branch from d829a3d to 9940cce Compare September 16, 2022 08:25

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 16, 2022 08:26 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 9940cce to f2fe40f Compare September 16, 2022 08:30

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 16, 2022 08:30 Inactive

36degrees reviewed Sep 16, 2022

View reviewed changes

colinrotherham added 2 commits September 18, 2022 00:02

Upgrade testing packages

6af2b30

Cache Jest transforms

6ef60de

colinrotherham force-pushed the upgrade-test-jest-percy branch from f2fe40f to 97f9135 Compare September 17, 2022 23:06

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 17, 2022 23:06 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 97f9135 to dc76549 Compare September 20, 2022 07:41

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 20, 2022 07:41 Inactive

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 20, 2022 07:46 Inactive

colinrotherham force-pushed the upgrade-test-jest-percy branch from 7cdecd0 to f3146cd Compare September 20, 2022 07:49

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 20, 2022 07:50 Inactive

36degrees reviewed Sep 20, 2022

View reviewed changes

colinrotherham added 5 commits September 20, 2022 09:58

Fix device pixel ratio differences in Puppeteer

045499d

Fix for mobile menu button visibility

fa89e57

Test was sometimes failing under load, potentially related to emulation mode viewport width not revealing the mobile menu immediately

Remove duplicated renderSass tests

5e550d8

See ./src/govuk/components/all.test.js

colinrotherham force-pushed the upgrade-test-jest-percy branch from f3146cd to 5e550d8 Compare September 20, 2022 08:58

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2850 September 20, 2022 08:59 Inactive

36degrees approved these changes Sep 20, 2022

View reviewed changes

colinrotherham merged commit bf9e54c into main Sep 20, 2022

Design System Sprint Board automation moved this from Needs review 🔍 to Done 🏁 Sep 20, 2022

colinrotherham deleted the upgrade-test-jest-percy branch September 20, 2022 09:11

colinrotherham mentioned this pull request Oct 21, 2022

Add cross-env-shell for Windows support #2938

Merged

colinrotherham removed their assignment Nov 8, 2022

colinrotherham mentioned this pull request Nov 8, 2022

Puppeteer tests can get run against outdated code #2242

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to Jest project setup + test package updates #2850

Improvements to Jest project setup + test package updates #2850

colinrotherham commented Sep 15, 2022 •

edited

36degrees left a comment

36degrees Sep 15, 2022

colinrotherham Sep 15, 2022

colinrotherham Sep 15, 2022

colinrotherham Sep 15, 2022 •

edited

36degrees Sep 15, 2022

colinrotherham Sep 15, 2022 •

edited

36degrees Sep 16, 2022

colinrotherham Sep 16, 2022 •

edited

colinrotherham Sep 16, 2022 •

edited

colinrotherham commented Sep 16, 2022 •

edited

colinrotherham commented Sep 16, 2022

36degrees Sep 16, 2022

36degrees Sep 16, 2022

colinrotherham Sep 16, 2022

colinrotherham Sep 20, 2022

36degrees Sep 16, 2022

36degrees Sep 16, 2022

colinrotherham Sep 20, 2022

36degrees Sep 16, 2022

36degrees Sep 16, 2022

36degrees Sep 16, 2022

colinrotherham Sep 16, 2022

colinrotherham Sep 20, 2022

36degrees Sep 20, 2022

colinrotherham Sep 20, 2022

36degrees left a comment

	- name: Run visual regression
	- name: Send screenshots to Percy

	displayName: 'Snapshot tests',
	displayName: 'Nunjucks macro tests',

	it(`${component}.scss renders to CSS without errors`, () => {
	return renderSass({
	file: `${configPaths.src}/components/${component}/_${component}.scss`
	})
	})

Improvements to Jest project setup + test package updates #2850

Improvements to Jest project setup + test package updates #2850

Conversation

colinrotherham commented Sep 15, 2022 • edited

Jest with --watch flag

36degrees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colinrotherham Sep 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colinrotherham Sep 15, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colinrotherham Sep 16, 2022 • edited

Choose a reason for hiding this comment

colinrotherham Sep 16, 2022 • edited

Choose a reason for hiding this comment

colinrotherham commented Sep 16, 2022 • edited

colinrotherham commented Sep 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

36degrees left a comment

Choose a reason for hiding this comment

colinrotherham commented Sep 15, 2022 •

edited

Jest with `--watch` flag

colinrotherham Sep 15, 2022 •

edited

colinrotherham Sep 15, 2022 •

edited

colinrotherham Sep 16, 2022 •

edited

colinrotherham Sep 16, 2022 •

edited

colinrotherham commented Sep 16, 2022 •

edited