Run unit tests only for plugins with changes

[ShyamGadde]

Summary

Fixes #1792

Relevant technical choices

• Run unit tests only for the plugin(s) affected by the changes in the pull request.
• Changes to the Optimization Detective plugin trigger tests for Embed Optimizer and Image Prioritizer due to their dependencies.
• All tests will run for all plugins when commits are added to the trunk branch.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.20%. Comparing base (26442b9) to head (c1c5b11). Report is 2 commits behind head on trunk.

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk    #1838      +/-   ##
==========================================
+ Coverage   71.21%   72.20%   +0.99%     
==========================================
  Files          86       92       +6     
  Lines        7000     7452     +452     
==========================================
+ Hits         4985     5381     +396     
- Misses       2015     2071      +56     

Flag	Coverage Δ		*Carryforward flag
auto-sizes	79.86% <ø> (?)
dominant-color-images	86.44% <ø> (?)
embed-optimizer	67.21% <ø> (?)
image-prioritizer	75.08% <ø> (?)
multisite	66.39% <ø> (-4.82%)	:arrow_down:	Carriedforward from db41189
optimization-detective	94.65% <ø> (?)
performance-lab	49.11% <ø> (?)
single	37.89% <ø> (-2.82%)	:arrow_down:	Carriedforward from db41189
speculation-rules	95.52% <ø> (?)
web-worker-offloading	88.88% <ø> (?)
webp-uploads	60.66% <ø> (?)

*This pull request uses carry forward flags. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions[bot]]

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ShyamGadde <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: phanduynam <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[westonruter]

How about we implement this change so that it only applies to pull requests, but for merges to our main branches it would still run all test suites? IMO this would be the best of both worlds:

This should already be the case with this logic:

https://github.com/WordPress/performance/blob/af6f8ff933a733bf9764514007a72356b3d5bb79/.github/workflows/php-test-plugins.yml#L83-L87

If any of the config files are changed or if the changes are not part of a PR (i.e. a push to trunk) then all plugins are tested.

[felixarntz]

@westonruter Thanks, I missed that. LGTM in that regard then 👍

In that case, this leaves the outstanding point @swissspidy raised about confusing codecov reports.

[ShyamGadde]

In that case, this leaves the outstanding point @swissspidy raised about confusing Codecov reports.

I think a good solution to this would be the carryforward flag. Codecov provides this flag to allow previously uploaded coverage reports to persist across runs, meaning untested files retain their last known coverage instead of being omitted from the report, which can lead to misleading drops in coverage. However, this won't work in the current setup. Right now, we are using two flags, single and multisite, and partial reports are being uploaded under these flags, which results in the PR comments showing a drop in reported code coverage (as untested files appear as uncovered). For carryforward to work, I believe no reports should be uploaded using the flag being carried forward; otherwise, the newly uploaded report will override the carried-forward coverage.

Another possible approach is to only show patch coverage in the PR comment, as that isn’t affected by partial testing. This would be the simpler solution, ensuring that PR reports remain clear and focused on the actual changes. Since full test runs still occur on merge commits and commits to trunk, the overall project coverage will still be recalculated periodically.

That said, the first approach (using carryforward) might still work with some adjustments. Instead of using just single and multisite, we could assign unique flags per plugin so that their coverage data is carried forward independently. That way, when a report for a particular plugin isn’t uploaded, its previous report would get carried forward automatically. (I'm still reviewing Codecov's documentation to confirm if this setup would work as expected.)

Screenshots:

1. With partial testing:
   
2. PR comment with only patch coverage:
   

cc @felixarntz @westonruter

[ShyamGadde]

So, after experimenting with Codecov's coverage reporting and reviewing its documentation, I’ve concluded that if we want to continue with partial testing, there are two possible ways to handle Codecov’s potentially confusing reports—depending on whether we want to display the project's overall coverage in the PR comment or not.

Option 1: Show Project Coverage in PR Comment

If we want to display project coverage in the PR comment, we need to use the carryforward flag to handle missing reports. But that would mean that we will not be able to use the flags such as single and multisite as we will be uploading partial reports with these flags when we don't test all the plugins which will lead to the confusing reports.

To address this, we can:

• Remove the single and multisite flags (also delete them from the Codecov dashboard).
• Instead, introduce plugin-specific flags and group single-site and multisite reports under those flags.

That way when we don't test a particular plugin the result will look something like in the below screenshot and even the project coverage which is displayed won't be affected when partial testing.

I personally find the reports (for both single and multisite) grouped by plugin flags more intuitive to read (because of how Codecov merges these reports) than the old reporting format, where we see 38.45% for single and 65.86% for multisite in PR comments as seen below:

Option 2: Show Only Patch Coverage in PR Comment

A simpler alternative is to only show patch coverage in PR comments. This won't show project-wide coverage fluctuations when running partial tests and keeps the report focused on the actual changes.

Additionally, if we were to only show "Patch coverage", we could:

• Limit Codecov's "Project Coverage" status check to only commits to the trunk branch, preventing confusing reports in Codecov's dashboard and GitHub's Actions runs. Since we run all tests on commits to trunk, everything should function normally there.
• Consider uploading reports without the single and multisite flags, since they are optional. This would prevent temporary drops in coverage reporting under these flags.

With this approach, the PR comment will look like this:

---

Given these options, assuming we still want to do partial testing, which approach would be preferable?

cc @felixarntz @westonruter

[westonruter]

@ShyamGadde Option 2 sounds like the better approach to me! Thank you for doing that research.

[felixarntz]

@swissspidy Since you raised some valid points in https://github.com/WordPress/performance/issues/1792#issuecomment-2630440480, it would be great to get your feedback whether this solution would address them.

[swissspidy]

@ShyamGadde Appreciate the thorough comparison here 💪 👍

While individual flags per plugin are tempting, I think we can start with the simpler option 2 for now and then see how well it works.

[phanduynam]

This issue has been resolved, please close it admin

[swissspidy]

@phanduynam No it's not, please stop spamming the repo with these comments.