Tests: Share JavaScript build assets across PHP workflows #60428
Conversation
sirreal
added
[Type] Code Quality
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
sirreal
changed the title
I explored this in #60430. It may be something we could cache beter, but just installing |
| - name: Docker debug information | ||
| run: | |
There was a problem hiding this comment.
Why do we have docker debug separate from general debug? Shall we combine this step and the next one?
There was a problem hiding this comment.
This logs the version of Docker installed within the runner image. The other debug steps below this one log the running containers, and then some debug detail about what's running within the containers.
I think it makes sense to keep them separate becuase they have different contexts, but don't feel strongly here.
There was a problem hiding this comment.
My question was about the following "General debug information" immediately following, not about the other docker logging like the running containers. It seems like the docker version could be printed there and we could save a step, it's mostly printing versions and it doesn't seem like there's any reason to isolate the docker version.
It's not too important and can be done in another PR if we'd like.
There was a problem hiding this comment.
I wanted to note that we did initially use this strategy in the first iteration of testing workflows for Core. But the savings noted here are much more substantial than in that approach. I've compared the latest run of this workflow for trunk and this PR, and below is the reported time usage.
It seems your math does indeed add up. I think this is worth a shot.
Co-authored-by: Jonathan Desrosiers <[email protected]>
…60428) Share the npm run build result across PHP unit test workflows. The PHP unit tests are one of the slowest parts of the unit tests and are run 19 times for every workflow run. The longest step in the runs is building the same JavaScript assets. By building once and reusing these assets we can save a significant amount of time. Around 1 hour of total CI runner time is saved for every run of the unit-test workflow. Co-authored-by: sirreal <[email protected]> Co-authored-by: desrosj <[email protected]>
What?
Share the
npm run buildresult across PHP unit test workflows.Why?
I noticed that the PHP unit tests tend to be one of the slowest parts of the unit tests and that one of the slowest steps tends to be building JavaScript assets. These assets are the same across all the PHP tests so if we can build them once and reuse them it may give us a significant speed up!
How?
Instead of running
npm run buildon every PHP unit test run, we run it 1 time and use upload/download actions to store and retrieve the assets. The PHP unit tests depend on the build, so we shift this part of the work earlier and the use the stored result in all of the runs.Compare the same job on this branch with the job on trunk.
The build step takes 3m 34s on trunk while the download step that replaces it on this branch takes 11s. The extra build job on this branch takes ~4:20, so we save around 3:20 x 19 PHP jobs - the additional 4:20 dedicated build job. Unless my math is off, this save 1:03:20 in total - 4:20 for the additional job. That's almost an hour of overall workflow time saved. If we compare the total time, that seems accurate. Usage on this branch reports 1:04:45 while a recent trunk build reports 2:11:50.
As a follow-up, we may be able to simplify the npm installation from the PHP jobs as well. After this change I think it's only used to run
wp-env.