PLATUI-3224 update jest #403

oscarduignan · 2024-09-13T15:46:03Z

update jest, refactored some test config as a result, had some broken tests with the timeout dialog which were hard to unpick/isolate with jsdom so I've refactored a lot of them into the browser-based tests instead, seems like we don't need --runInBand (run tests in sequence) so they should run faster now, but we'll see if that holds up in jenkins.

I think I've still got a bit of tidying up to do before this is totally ready for review.

most of this is moving around test coverage and trying to simplify test code so it's easier to maintain in future 🤞

oscarduignan · 2024-09-13T15:47:03Z

config/jest-setup.js

-
-const { toHaveNoViolations } = require('jest-axe');
-
-expect.extend(toHaveNoViolations);


didn't seem to be needed

oscarduignan · 2024-09-13T15:47:28Z

.eslintrc.json

+    "browser": true,
+    "context": true,
+    "puppeteerConfig": true,
+    "jestPuppeteer": true


stops ide warnings about globals

oscarduignan · 2024-09-13T15:50:00Z

jest.config.mjs

+        '**/*browser.test.js',
+      ],
+    },
+  ],


previously the environment of a test was configured by comments at the top of test files, there were some limits of what custom options you could specify that way so instead I've opted to rename tests with different suffixes. This matches the approach that govuk-frontend uses too https://github.com/alphagov/govuk-frontend/blob/main/jest.config.mjs

oscarduignan · 2024-09-13T15:50:34Z

jest.config.mjs

+    },
+  ],
+
+  modulePathIgnorePatterns: ['<rootDir>/package'],


stops us getting a warning from jest haste map

oscarduignan · 2024-09-13T15:54:46Z

lib/browser-tests/puppeteer-helpers.js

+      window.clock = sinon.useFakeTimers(${JSON.stringify(config)})
+    }
+  `);
+}


we were using sinon before - but a part of it only, and to do that we had to compile the extracted part from the browser, I discovered that if we just use the whole sinon library, it's npm package has a compiled js file ready to use in the browser, so no more need for that extra build step in our tests

oscarduignan · 2024-09-13T15:55:32Z

lib/browser-tests/puppeteer-helpers.js

+    req.respond({ contentType: 'text/html', body });
+    page.setRequestInterception(false);
+  });
+  await page.goto('http://localhost:3000/'); // if ever flaky, waitUntil networkidle0 (js loaded)


some of the tidying up needed is to replace anywhere I've used hardcoded port with the port from config

oscarduignan · 2024-09-13T15:57:52Z

src/all.jsdom.test.js

+      expect(BackLinkHelper.prototype.init).toHaveBeenCalledTimes(2);
+    });
+  });
+});


this replaces src/all.test.js it's the same thing renamed and refactored but changed enough so that it's not picked up as a rename

oscarduignan · 2024-09-13T15:58:25Z

package.json

-    "testRunner": "jest-jasmine2",
-    "globalSetup": "./lib/puppeteer/setup.js",
-    "globalTeardown": "./lib/puppeteer/teardown.js"
-  }


now configured in jest.config.mjs

package.json

oscarduignan · 2024-09-13T16:02:05Z

src/components/currency-input/__snapshots__/jsdom.test.js.snap

I think something is up with these currency component snapshots- doesn't seem to be actually rendering the component anymore - can't remember if that's because of changes to currency input which need to be reflected in tests (do we need them anymore? future tech debt ticket to be created)

oscarduignan · 2024-09-13T16:03:33Z

src/components/timeout-dialog/timeout-dialog.jsdom.test.js

I have left some tests here that were checking things like argument validation and message overrides - but moved the behavioural stuff into browser tests

oscarduignan · 2024-09-13T16:03:53Z

src/components/timeout-dialog/timeout-dialog.browser.test.js

this is where I extracted lots of timeout dialog tests into

oscarduignan · 2024-09-13T16:04:46Z

src/components/timeout-dialog/timeout-dialog.browser.test.js

+    await session.close();
+  });
+
+  // TODO because currently it would actually log you out straight away, this is an edge case


might be an edge case that in practice can't happen (I don't know if session activity could be delayed like this, so left this as a skipped test to show how it currently behaves)

oscarduignan · 2024-09-13T16:07:38Z

src/components/timeout-dialog/timeout-multiple-tabs.browser.test.js

another one where it already existed but renamed and refactored enough that it thinks it's new file

oscarduignan · 2024-09-13T16:08:41Z

src/components/user-research-banner/user-research-banner.browser.test.js

 const configPaths = require('../../../config/paths.json');

 describe('/components/user-research-banner', () => {
-  let page;
-  const url = `http://localhost:${configPaths.ports.test}/components/user-research-banner/default/preview`;
+  const url = `http://localhost:${configPaths.ports.app}/components/user-research-banner/default/preview`;


I didn't think we needed a separate test port - we can just reuse the app one - and the jest can skip spinning up it's own dev server if ones already running

oscarduignan · 2024-09-13T16:09:32Z

src/components/user-research-banner/user-research-banner.browser.test.js

@@ -28,10 +17,6 @@ describe('/components/user-research-banner', () => {
    await page.deleteCookie({ name: 'mdtpurr' });
  });

-  afterAll(async () => {
-    await page.close();
-  });


in the browser-test jest-setup it resets the page before each test now to save stuff like this being duplicated around

tasks/gulp/backstop.js

oscarduignan · 2024-09-13T16:18:51Z

...stop_default_timeout-dialog_timeout-warnings-not-synchronised_example_0_document_0_phone.png

wanted a new example for some tests with specific timeout length, could have refactored it to not use the examples and just render html into the test - which would be better but 🤷‍♂️

JoPintoPaul · 2024-09-16T11:14:05Z

lib/browser-tests/puppeteer-helpers.js

+  await clockTick(page, s * 1000);
+}
+
+const preloadGovukFonts = fs.readdirSync(


Is this so that the test is not slowed / flaky due to slow font loading?

yup, for some of the timeout dialog tests there isn't any text rendered using the gov fonts on page load, only when the dialog triggers, and I guess css fonts are deferred by default so that's when the request is made to load them. Some of the timeout dialog tests intercept timeout / sign-out request to simulate a slow response and it was triggering on the fonts, so I copied over the preload code from the example app that's used for visual regression tests where we were getting some screenshots without fonts

hmrc-frontend/app/views/layouts/includes/head.html

Line 7 in 5d05b9c

<link rel="preload" href="/assets/govuk/fonts/{{font}}" as="font" type="font/woff2" crossorigin>

JoPintoPaul · 2024-09-16T11:22:43Z

src/components/timeout-dialog/timeout-dialog.browser.test.js

+    await expect(page.url()).toMatch('/signed-out-early');
+  });
+
+  // TODO at the moment it doesn't, should we change test or implementation?


This is an interesting one, worth capturing somewhere?

yup, could probably be a tech debt ticket, I'll create one and link it here

currently browser tests are disabled moving to use jest-puppeteer add a browser test as an example refactor timeout dialog tests fix after webjar build test

oscarduignan · 2024-09-17T15:18:15Z

src/components/back-link-helper/back-link-helper.yaml

I don't think we need VRT of the examples for this - just one example with the correct attribute set is enough

oscarduignan · 2024-09-17T15:18:32Z

src/components/back-link-helper/back-link-helper.nojs.test.js

was actually renamed and refactored, see above file

oscarduignan · 2024-09-17T15:20:13Z

src/components/back-link-helper/back-link-helper.browser.test.js

refactored the vrt examples into the browser tests, behaviour is a bit clearer like this I think, and with the examples we get extra screenshots being taken that we don't need

oscarduignan commented Sep 13, 2024

View reviewed changes

package.json Show resolved Hide resolved

oscarduignan commented Sep 13, 2024

View reviewed changes

tasks/gulp/backstop.js Outdated Show resolved Hide resolved

oscarduignan force-pushed the jest-update branch from 2a69040 to ab96a08 Compare September 13, 2024 16:16

oscarduignan commented Sep 13, 2024

View reviewed changes

JoPintoPaul reviewed Sep 16, 2024

View reviewed changes

PLATUI-3224 update jest

ca1cf87

currently browser tests are disabled moving to use jest-puppeteer add a browser test as an example refactor timeout dialog tests fix after webjar build test

oscarduignan force-pushed the jest-update branch 2 times, most recently from cafa2d0 to d0c7bcd Compare September 17, 2024 15:13

PLATUI-3224: introduce url helper to get url for example preview

c585e5d

oscarduignan force-pushed the jest-update branch from d0c7bcd to c585e5d Compare September 17, 2024 15:16

oscarduignan commented Sep 17, 2024

View reviewed changes

remove packages we no longer need

a8d67fe

oscarduignan force-pushed the jest-update branch from 86ee4ae to a8d67fe Compare September 17, 2024 15:25

oscarduignan marked this pull request as ready for review September 17, 2024 15:28

JoPintoPaul approved these changes Sep 18, 2024

View reviewed changes

oscarduignan merged commit f064b96 into main Sep 24, 2024
1 check passed

oscarduignan deleted the jest-update branch September 24, 2024 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PLATUI-3224 update jest #403

PLATUI-3224 update jest #403

oscarduignan commented Sep 13, 2024 •

edited

Loading

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

oscarduignan Sep 13, 2024

JoPintoPaul Sep 16, 2024

oscarduignan Sep 16, 2024

JoPintoPaul Sep 16, 2024 •

edited

Loading

oscarduignan Sep 16, 2024

oscarduignan Sep 17, 2024

oscarduignan Sep 17, 2024

oscarduignan Sep 17, 2024


		const { toHaveNoViolations } = require('jest-axe');

		expect.extend(toHaveNoViolations);

PLATUI-3224 update jest #403

PLATUI-3224 update jest #403

Conversation

oscarduignan commented Sep 13, 2024 • edited Loading

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

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

JoPintoPaul Sep 16, 2024 • edited Loading

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

oscarduignan commented Sep 13, 2024 •

edited

Loading

JoPintoPaul Sep 16, 2024 •

edited

Loading