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

Clear out and remove /tests directory #1433

Open
felixarntz opened this issue Aug 5, 2024 · 3 comments · May be fixed by #1804
Open

Clear out and remove /tests directory #1433

felixarntz opened this issue Aug 5, 2024 · 3 comments · May be fixed by #1804
Assignees
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Comments

@felixarntz
Copy link
Member

After #1262, there are still some files in the root /tests directory, which IMO seem a bit misplaced there. Mostly, the existence of the directory could lead to confusion for where tests should be added.

Some thoughts:

  • I think the test images should be moved to tests/data directories of the specific plugins that use them. If multiple plugins use these images, no harm to duplicate them across those locations. I think it makes sense for every plugin to self-contain its test data.
  • The Optimization Detective test helpers should probably be moved to somewhere within /plugins/optimization-detective/tests.
  • The boostrap.php file could be moved to our /tools somewhere? Maybe a new directory /tools/phpunit? It's a configuration helper for PHPUnit, similar to how there's already configuration helpers for e.g. PHPCS and PHPStan in that directory.

cc @westonruter @thelovekesh

@felixarntz felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure labels Aug 5, 2024
@westonruter westonruter moved this to Not Started/Backlog 📆 in WP Performance 2024 Aug 5, 2024
@westonruter
Copy link
Member

SGTM.

For the images, we may want to shrink them down to speed up the tests as well.

@westonruter
Copy link
Member

Otherwise, the only reason for a /tests directory would be to introduce test suites for combinations of plugins as discussed in #1012 (comment) and #1262 (comment). But seeing as we don't have that now, we can always add it back later if need be. However, as it stands right now the Enhanced Responsive Images and Embed Optimizer plugins do not have a strict plugin dependency on Optimization Detective. Nevertheless, they bootstrap Optimization Detective (and Image Prioritizer) in order to test their integrations which are available when they are active. It would be a bit better if the test suites for these plugins could run without the other optional plugins active, and then run again with these plugins active. This would ensure they work as expected in both cases. So this could be a case where a /tests directory could be used: There could be test suites in /tests/embed-optimizer-with-optimization-detective and /tests/auto-sizes-with-image-prioritizer which which are configured to bootstrap the multiple plugins, run the tests for them all, and then run any additional tests written specifically to test their integrations. cc @joemcgill

@felixarntz
Copy link
Member Author

Makes sense to me. Regarding a combination of plugins, if we want to test that, I think the /tests directory is fine, however for the more specific case of testing a plugin with its dependency, I don't think we need /tests. The purpose of those tests will probably be to test functionality of the dependent plugin while the dependency plugin is active, so I think this should rather go into the specific plugin's tests directory. It could be a second test suite for that plugin, where the dependency (e.g. Optimization Detective) is active.

@sstopfer sstopfer moved this from Not Started/Backlog 📆 to In Progress 🚧 in WP Performance 2024 Aug 26, 2024
@macka61 macka61 moved this from In Progress 🚧 to To Do 🔧 in WP Performance 2024 Oct 7, 2024
@ShyamGadde ShyamGadde linked a pull request Jan 16, 2025 that will close this issue
@ShyamGadde ShyamGadde moved this from To Do 🔧 to In Progress 🚧 in WP Performance 2024 Jan 16, 2025
@ShyamGadde ShyamGadde self-assigned this Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature
Projects
Status: In Progress 🚧
Development

Successfully merging a pull request may close this issue.

3 participants