Fix for flaky Windows tests preventing deployment #2647
Conversation
Whilst running `jest --watch` we were doing a full build on every edit
✅ You can preview this change here:
To edit notification comments on pull requests, go to your Netlify site settings. |
| "test": "npm run lint && jest", | ||
| "lint": "npm run lint:js && npm run lint:scss && npm run lint:html", | ||
| "lint": "npm run lint:js && npm run lint:scss && npm run lint:html --ignore-scripts", |
There was a problem hiding this comment.
When running the "All in one" task npm test we have already built the site using pretest
So I've added --ignore-scripts to skip prelint:html to avoid a double build
There was a problem hiding this comment.
question If we're doing this to avoid the double build when running test, because both pretest and prelint:html run a build, would this addition be better suited to the test command? Can it run npm run lint -- --ignore-scripts to append ignore-scripts to the lint command (and as such to its last lint:html command)? No worries if it can't though.
There was a problem hiding this comment.
Does it work? Happy to switch it
Only worry is "lint" being reordered in future and the flag goes to the wrong script
There was a problem hiding this comment.
Seems to work, yup (🤯), but good shout on the reordering. Feels odd that, when run through lint, lint:html won't build, but when run on its own it will. If it becomes an issue we can always adjust then.
There was a problem hiding this comment.
Yeah maybe aligning it with govuk-frontend was the wrong move?
Where only "test" runs a build via "pretest"
There was a problem hiding this comment.
I'm not sure I understand your last answer, sorry 😔
There was a problem hiding this comment.
Ah sorry. I mean the linting tasks on govuk-frontend don't run a build, so I copied that here
Might be worth trying to tighten up what runs when
|
The branch protection status checks will need changing These don't exist in the PR anymore: Build & Test (ubuntu-latest)
Build & Test (windows-latest) |
|
@romaricpascal One thing I left out from Click into "Cache build" and you'll see a warning is logged: The warning is logged because we already cached it on the previous run If that's an issue I can put the conditions in we use in # Skip install when build is already cached
if: steps.build-cache.outputs.cache-hit != 'true' |
|
Oh, I wasn't aware of that. If we already have a solution on |
2420ed4 to
4073f53
Compare
Windows tests are running slow and flaky
4073f53 to
2474a14
Compare
@romaricpascal Done, here's one that's been re-run: If you click into the Build job you'll see the skipped steps |
This PR copies
govuk-frontendby making Windows tests optional:Why? Since the
puppeteer@1.14.0puppeteer@18.2.1upgrade Windows tests runs keep timing out:This PR tackles the issue with:
Only build once per test run via
pretestThis also fixes
jest --watchrunning a full rebuild on every editRun GitHub tasks in parallel
Speeds up our GitHub workflow and gives each task a separate container
Run GitHub tasks in Linux by default
Windows test runs are now optional but may return