Puppeteer tests can get run against outdated code

[lfdebrux]

Cause

Checking out a branch and running the tests without first running the review app will run the tests against whatever was last built into /public – this is likely to be whatever the previous branch was.

Frontend Puppeteer tests rely on the review app in order to run correctly.

It is tightly coupled to the review app's structure and the examples we use in the review app.

Running tests starts its own server on a separate port, but the tasks that actually compile the Javascript or Sass from src/ for the review app are not run.

Consequences

• This could create a false positive where tests are seen passing but in reality they would not pass on CI

• This could also create a false negative where tests are not passing despite passing on CI or when the developer previously ran them

• This is hard to debug – it happens infrequently and basically relies on the developer remembering that 'this is a thing' and/or coincidentally starting the review app

• This would be especially hard for contributors to understand

• It would be more difficult to make changes to the review app without also having to update the tests. For example, if we wanted to change the URL structure or the way that examples are embedded in the page.

• The review app is intended to be used to verify the visual appearance of components in different contexts and with different configuration. We have an (unwritten) rule that examples that look and behave the same but differ e.g. by having additional example classes or data attributes should not be included in the review app, and this functionality should be tested using unit tests at the template level. Because Puppeteer based tests rely on the review app, we could find ourselves in situations where we have to break this rule in order to be able to effectively test functionality.

Impact of debt

HIGH due to potential for developer confusion, especially for contributors.

Effort to pay down

MEDIUM

Overall rating:

MEDIUM?

[lfdebrux]

Copied from https://trello.com/c/gSc9dtsv/16-puppeteer-tests-can-get-run-against-outdated-code

[lfdebrux]

I don’t think it’s clear enough from the ticket what the fix is for this problem :thinking_face: Maybe that knowledge already exists in someone’s head, in which case if we could capture it that would make the ticket easier, but right now it feels a bit like a potential rabbit hole.

Might be worth discussing at a future dev catch-up, if time permits.

[colinrotherham]

Looks like this was fixed accidentally by compiling ./public in 8bc8973 via:

• Improvements to Jest project setup + test package updates #2850

We additionally added build jobs for ./package and ./dist in 016f7fe via:

• Fix stray let in I18n preventing uglify (into main) #2870

It's still possible to bypass the initial review application compile by calling npx jest directly but see:

"pretest": "npm run build:compile",
"test": "jest --color --ignoreProjects \"Gulp tasks\" --maxWorkers=50%",

As long as tests are run with npm t, npm test or npm run test you'll trigger the "pretest" npm script