For discussion: Strategies to reduce GitHub workflow runs (+ Percy)

[colinrotherham]

Just adding some GitHub Action workflow tweaks here

Ideas so far

Cancel in-progress GitHub workflows when pushed again Forgot a change? Pushing up a second time with cancel the 1st test run

Remove push event from GitHub Actions branch workflows Prevents duplicate runs as pull_request event already fires for PR pushes

Skip tests for documentation changes Don't need to run JavaScript tests when only docs/**/*.md files have changed (??)

Move “All components” template tests out of Puppeteer environment Don't lose time spinning up Chromium (headless) to run HTML validation checks etc

Only take “JavaScript disabled” screenshots for components with JS Skip "JavaScript disabled" tests for components lacking *.mjs files as screenshots will be identical

Before

1. Multiple workflow [pull_request, push] events for PRs
2. Screenshots all components with JavaScript enabled
3. Screenshots all components with JavaScript disabled

GitHub Workflow: 14 checks total (7 checks per run) Screenshots (JavaScript enabled): 64 images (32 per run) Screenshots (JavaScript disabled): 64 images (32 per run)

Time:        89.322 s, estimated 96 s

After

1. Single workflow pull_request events for PRs
2. Screenshots all components with JavaScript enabled
3. Screenshots "JS capable" components with JavaScript disabled

GitHub Workflow: 7 checks total Percy screenshots (JavaScript enabled): 32 images Percy screenshots (JavaScript disabled): 11 images

Time:        58.233 s, estimated 96 s

[colinrotherham]

@36degrees Morning! This commit for “JavaScript disabled” screenshots follows your idea in https://github.com/alphagov/govuk-frontend/pull/2871

I've added a getFiles() directory listing helper to filter only components with ${component}.mjs, but we can run it before all the tests so once Puppeteer opens the Chromium window it stays rapid

[colinrotherham]

Thanks @romaricpascal, cheers for the feedback

Can we narrow down our Jest projects: [] list (by directory/filename) in another PR?

I've made sure groups don't overlap and setup/teardown scripts only run where necessary in this change: https://github.com/alphagov/govuk-frontend/commit/8bc8973bb136451bbf6369fed425951d8802b03b#diff-1e058ca1442e46581b13571fb8d261f9e1f5657e26c96634d4c1072f0f0347f1

[romaricpascal]

Can we narrow down our Jest projects: [] list (by directory/filename) in another PR?

Yeah, that'd be fine by me so we get this one moving and tidy up naming conventions separately 😄

[colinrotherham]

Also a note about the Add Sass test run matrix config commit https://github.com/alphagov/govuk-frontend/pull/2878/commits/843ad1683ec0bcea7cded5b703976e30e82855dc

This removes "boilerplate" GitHub Action workflow code so new tests are only extra array items:

- name: Dart Sass v1.0.0
  node-version: 8 # v8 required for sass v1.0.0

  run: |
    npm install -g [email protected]
    sass --version
    time sass src/govuk/all.scss > /dev/null

[colinrotherham]

Thanks for this, @colinrotherham 🙌🏻

Generally looks good, but there is a lot going on in this PR – I might be missing something but the changes to the workflows feel pretty much unrelated to lots of the changes to the tests, for example.

The work is logically broken down into sensible commits though, so I don't think it's worth splitting it out at this stage.

It's really helpful to have extra context added through the review comments, but ideally I think at least some of it should be in the commit messages, which don't always explain why a change is happening.

Thanks @36degrees

The Sass workflow matrix has gone, but I've explained why we need workflow_dispatch

I've pushed up again with commit comments too

Will take another look at naming conventions

[colinrotherham]

Will take another look at naming conventions

@36degrees Ready for you to take a look at again, with one last question about all.test.js (Percy)

It's still the original filename (before this PR) but happy to change it if it's a good time to do so 👍

[colinrotherham]

Morning @36degrees

I've gone through and split out all the commits, explaining what's happening and why

If you can take another look please 😊