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.

Sign up for GitHub

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

Jump to bottom

Unit testing should be skipped for plugins not related to changes in a pull request #1792

Open
westonruter opened this issue Jan 12, 2025 · 4 comments · May be fixed by #1838
Open

Unit testing should be skipped for plugins not related to changes in a pull request #1792

westonruter opened this issue Jan 12, 2025 · 4 comments · May be fixed by #1838
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@westonruter
Copy link
Member

Feature Description

When opening a pull request that changes files to one plugin, only the unit tests for that plugin should run. This will greatly speed up our workflows (and reduce wasted compute), especially now that code coverage is being performed. That said, would it be problematic if PRs didn't include coverage reports for all plugins?

Note that a PR that modifies Optimization Detective should also cause the tests for Embed Optimizer and Image Prioritizer to also run, since they are dependent on that plugin.

Naturally, all tests would run for all plugins for commits added to trunk.

This has been suggested before by @thelovekesh, but I don't remember where.
@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure labels Jan 12, 2025
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Jan 12, 2025
@ShyamGadde ShyamGadde linked a pull request Jan 30, 2025 that will close this issue
Open
@swissspidy
Copy link
Member

When opening a pull request that changes files to one plugin, only the unit tests for that plugin should run. This will greatly speed up our workflows (and reduce wasted compute), especially now that code coverage is being performed.

Our tests currently run in 4-5 minutes, with coverage 7-8. That isn't really much.

That said, would it be problematic if PRs didn't include coverage reports for all plugins?

That might indeed lead to confusing reports on codecov

@felixarntz
Copy link
Member

I left additional thoughts on this while reviewing the PR in #1838 (review). I'm not sure we would want to do this always - it makes sense to me for PRs, but we should also keep testing the whole codebase regularly.

What @swissspidy says above is another concern, so it seems we should discuss those considerations first before moving forward with the PR.

@westonruter westonruter changed the title Unit testing should be skipped for plugins not related to changes in a pull request Unit testing should be skipped for plugins not related to changes in a pull request Feb 3, 2025
@westonruter
Copy link
Member Author

Our tests currently run in 4-5 minutes, with coverage 7-8. That isn't really much.

No, but if we reduce the time to run tests from 8 minutes (for all 9 plugins) down to just <1 minute (for 1 plugin), that would be very nice.

@westonruter
Copy link
Member Author

I'm not sure we would want to do this always - it makes sense to me for PRs, but we should also keep testing the whole codebase regularly.

This should already be the case as I also commented on the PR: #1838 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature
Projects
WP Performance 2024
Status: Not Started/Backlog 📆
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Run unit tests only for plugins with changes ShyamGadde/wp-performance
3 participants
@westonruter @swissspidy @felixarntz