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

Jetpack Boost: Split and Parallelize Jetpack Boost Tests #32686

Merged
merged 42 commits into from
Aug 30, 2023

Conversation

pyronaur
Copy link
Member

Proposed changes:

  • Split Boost tests into multiple directories (base, critical-css, modules)
  • Run Jetpack Boost tests in parallel
  • Updated file paths in test files to reflect new directory structure

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?
    N/A

Jetpack product discussion

N/A

Does this pull request change what data or activity we track or use?

N/A

Testing instructions:

  • Run all Jetpack Boost tests

@github-actions github-actions bot added [Plugin] Boost A feature to speed up the site and improve performance. [Tests] Includes Tests Actions GitHub actions used to automate some of the work around releases and repository management E2E Tests labels Aug 25, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 25, 2023

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.


Boost plugin:

  • Next scheduled release: September 5, 2023.
  • Scheduled code freeze: August 28, 2023.

@github-actions github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Aug 25, 2023
@pyronaur pyronaur force-pushed the boost/parallel-e2e branch 2 times, most recently from 87e08c0 to b4ab52f Compare August 28, 2023 12:21
pyronaur and others added 17 commits August 29, 2023 17:24
* Create both the test and build matrices in `create-test-matrix`.
* Generate an empty build matrix if no build is needed.
* Don't generate build entries for "atomic" suites.
* Check build matrix for having more than one path.
* Skip `build-projects` job if the matrix is empty.
* Remove suite check in `build-projects` build step since we do it in `create-test-matrix` now.
* We have to run `e2e-tests` even if `build-projects` is skipped, but not it `create-test-matrix` was skipped.
* Skip `e2e-tests` if test matrix is empty, otherwise GH complains.
* Only restore build artifact if we need it.
* Complain if build job wasn't skipped when it should have been.
* Complain if artifact restore didn't when it should have.
* Revert changes to reporting jobs' conditions.
Copy link
Contributor

@haqadn haqadn left a comment

Choose a reason for hiding this comment

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

I am giving it a ✅ because it looks like the changes worked, the reviewers were addressed, and I couldn't find any obvious issues. However, I do not have good knowledge of the e2e-tests.yml configuration. So, if in doubt, please get another ✅

image

@pyronaur pyronaur merged commit 7a5b760 into trunk Aug 30, 2023
54 checks passed
@pyronaur pyronaur deleted the boost/parallel-e2e branch August 30, 2023 09:38
@github-actions github-actions bot removed [Status] Needs Team Review [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Aug 30, 2023
@anomiex
Copy link
Contributor

anomiex commented Aug 30, 2023

Looks like this broke the repository_dispatch runs, e.g. https://github.com/Automattic/jetpack/actions/runs/6024492530 (although some of them seem to have been broken in different ways before).

I'll look at making a followup PR.

anomiex added a commit to anomiex/jetpack that referenced this pull request Aug 30, 2023
During all the code rearranging in Automattic#32686, a piece of code that was only for the workflow_run version wound up being incorrectly run for repository_dispatch as well.
anomiex added a commit that referenced this pull request Aug 30, 2023
During all the code rearranging in #32686, a piece of code that was only
for workflow_run wound up being incorrectly run for repository_dispatch
as well.
anomiex added a commit that referenced this pull request Aug 30, 2023
During all the code rearranging in #32686, a piece of code that was only
for workflow_run wound up being incorrectly run for repository_dispatch
as well.
TimBroddin pushed a commit that referenced this pull request Sep 5, 2023
…ts (#32686)

* Split Boost tests into directories

* Run Jetpack Boost tests in parallel

* Add changelog entry.

* Invalid colon

* [test] cache artifacts & build artifacts

* Revert "[test] cache artifacts & build artifacts"

This reverts commit f911559.

* Introduce build groups

* Add strategy

* Correct artifact directory

* Cache build output

* Use v3 cache

* Replace artifacts with cache

* Attempt to fix paths

* Use changed files as the cache hash

* Revert "Use changed files as the cache hash"

This reverts commit 492d868.

* Fix cache paths

* Cache the whole matrix path directory

* Skipped jobs are ok

* Fix the key

* Cache the whole directory, not just matrix path

* Always run slack notification and test report

* Rename "Setup" to "Base"

To avoid conflicting naming throughout the workflow runs. "Setup" is kind of confusing.

* Revert "Always run slack notification and test report"

This reverts commit d5d5125.

* Remove always()

* Matrix over concurrency

* Ensure build cache

* Group Jetpack projects into jetpack-core

* Restore repo check

* build-matrix -> matrix

* Fetch the build matrix

* Don't need the project name here

* Clean up job flow

* Create both the test and build matrices in `create-test-matrix`.
* Generate an empty build matrix if no build is needed.
* Don't generate build entries for "atomic" suites.
* Check build matrix for having more than one path.
* Skip `build-projects` job if the matrix is empty.
* Remove suite check in `build-projects` build step since we do it in `create-test-matrix` now.
* We have to run `e2e-tests` even if `build-projects` is skipped, but not it `create-test-matrix` was skipped.
* Skip `e2e-tests` if test matrix is empty, otherwise GH complains.
* Only restore build artifact if we need it.
* Complain if build job wasn't skipped when it should have been.
* Complain if artifact restore didn't when it should have.
* Revert changes to reporting jobs' conditions.

* Try caching only added files

* Source funcs file before the cd

* Remove workflow_run stuff, trigger was removed in #27623 last year

* Need to cd back to GITHUB_WORKSPACE for the after `find` anyway

* Can't pass too long of a list to actions/cache/save. So remove unchanged files insead.

* Can't delete `./.github`, it breaks "Post Setup tools".

* Sigh, xargs

* `path` needs to match, sigh

* Remove the extra dollar 💸

---------

Co-authored-by: Brad Jorsch <brad.jorsch@automattic.com>
TimBroddin pushed a commit that referenced this pull request Sep 5, 2023
During all the code rearranging in #32686, a piece of code that was only
for workflow_run wound up being incorrectly run for repository_dispatch
as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Actions GitHub actions used to automate some of the work around releases and repository management E2E Tests [Plugin] Boost A feature to speed up the site and improve performance. [Status] Needs Test Review [Tests] Includes Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants