Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split up test jobs so that they can be run concurrently #1056

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

frankieroberto
Copy link
Contributor

@frankieroberto frankieroberto commented Oct 23, 2024

Currently all the tests run one after each other, taking just under 5 minutes in total.

Aim here is to run them concurrently, which should roughly half the time it takes to run them all (slowest one is visual regression tests which take about 2.5 mins).

In addition they’ll all get reported on the PR separately, which should make it easier to quickly see which one failed, rather than having to dig into the actions log to work it out.

Before

Screenshot 2024-10-23 at 19 13 53

After

Screenshot 2024-10-23 at 19 20 42

Copy link
Contributor

@paulrobertlloyd paulrobertlloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@paulrobertlloyd
Copy link
Contributor

Not sure why this can’t be merged; something about needing all required statuses?

@frankieroberto
Copy link
Contributor Author

@paulrobertlloyd yeah, will need an admin to update the branch protection rules, removing the requirement for the current general “Pull request” job to pass and instead adding the 4 new ones.

@roshaanbajwa is this something you could do?

Also be good to check if the names for all 4 jobs make sense? I wasn’t sure if “JavaScript unit test” and “Visually regression tests” were the right terms or not?

@frankieroberto
Copy link
Contributor Author

Possibly the overall workflow should be renamed from “Pull request” too?

The names only affect the interface I think, but given these tests will also all run if any internal or outside new contributors open a pull request, the less confusing they are the better? (Although we can also help anyone who gets stuck)

@anandamaryon1
Copy link
Collaborator

Seems very sensible to me 👍

@roshaanbajwa is on leave now but I'm sure we can get this checked and merged on his return.

@frankieroberto
Copy link
Contributor Author

@anandamaryon1 @roshaanbajwa the settings should be on this page: https://github.com/nhsuk/nhsuk-frontend/settings/branches under 'branch protection rules'.

@anandamaryon1 anandamaryon1 merged commit 1ba3a91 into main Nov 7, 2024
5 checks passed
@anandamaryon1 anandamaryon1 deleted the split-up-test-jobs branch November 7, 2024 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants