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]

Codecov Report

All modified and coverable lines are covered by tests ✅

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%)	⬇️	Carriedforward from db41189
optimization-detective	94.65% <ø> (?)
performance-lab	49.11% <ø> (?)
single	37.89% <ø> (-2.82%)	⬇️	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.

☔ View full report in Codecov by Sentry.
📢 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.

[ShyamGadde]

This limits the directory output to a maximum depth of 2. For example, plugins/<plugin-name>/tests will be returned as plugins/<plugin-name>.
Since dir_names: true ensures only unique directories are listed, combining it with dir_names_max_depth: 2 allows us to extract a clean list of changed plugin names. This avoids additional logic in the next step for determining modified plugins.

[ShyamGadde]

Currently, optimization-detective is the only plugin with dependencies, so we could simplify this by checking for it explicitly instead of maintaining a general dependency structure. However, I think this approach keeps things flexible if more dependencies are introduced in the future.

[github-actions]

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]>

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

[westonruter]

PhpStorm (via Shellcheck) is flagging this line:

Arrays implicitly concatenate in [[ ]]. Use a loop (or explicit * instead of @).
See SC2199.

[ShyamGadde]

Fixed it in 11949ac.

[felixarntz]

@ShyamGadde This looks great to me so far, just one minor comment.

Before we merge this, I would like to discuss though under which conditions we should do this. To me, it makes sense to only run selected unit tests for PRs, to speed up the process. But running the whole test suite also has value, e.g. to warn us early in case something breaks somewhere else where we didn't change any files - which can happen e.g. based on WordPress Core updates.

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:

• Running tests against PRs would be more efficient, faster and cost less resources.
• Running tests against code merged into trunk and release branches would still be ensured to be more comprehensively tested.

It probably wouldn't be a huge lift to change that, we could make the step that determines the changed files and plugins conditional to pull_request events and otherwise return the list of all plugins.

WDYT? cc @westonruter @thelovekesh

[felixarntz]

Maybe call this PLUGIN_DEPENDENTS to clarify? In this case, Optimization Detective is a dependency of the other two, but the other two are dependents of Optimization Detective.

[ShyamGadde]

That makes sense. Updated it in 90a5ab7.

[felixarntz]

Marking this as "Request changes" until we resolve the outstanding discussion points on the approach in #1792.

[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:

performance/.github/workflows/php-test-plugins.yml

Lines 83 to 87 in af6f8ff

	if [[ "${{ github.event_name }}" == "push" || "${{ steps.changed-files.outputs.config_any_changed }}" == "true" ]]; then
	ALL_CHANGED_PLUGINS=($(ls plugins))
	echo "all_changed_plugins=${ALL_CHANGED_PLUGINS[*]}" >> $GITHUB_OUTPUT
	exit 0
	fi

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]

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.

[ShyamGadde]

I noticed that the threshold for patch coverage is set to 80%, which (based on Codecov’s docs) means that coverage can drop by up to 80% while still passing. I'm wondering if this was intentional—perhaps it was meant to ensure a minimum of 80% coverage rather than allowing such a large drop?

Additionally, since target isn't explicitly set, it defaults to the project coverage from the pull request base. That results in PR checks displaying something like:

Would it make sense to set target: 100% for patch coverage, with a lower threshold (maybe 10-20%), to ensure new changes are reasonably covered? Just wanted to check if this aligns with the intended behavior.

[swissspidy]

I'm wondering if this was intentional—perhaps it was meant to ensure a minimum of 80% coverage rather than allowing such a large drop?

I think so, yes. The naming is a bit confusing.

So maybe threshold: 20% or target: 80%?

[felixarntz]

@swissspidy Since you raised some valid points in #1792 (comment), 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.

[ShyamGadde]

@westonruter Would you prefer that we display the carried forward flags in the PR comment, as shown below, or would you rather exclude them?

Ref: https://docs.codecov.com/docs/carryforward-flags#advanced-configuring-carryforward-flags-in-the-code-host

[westonruter]

I suppose it doesn't hurt to include?