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

Replace wp-env with install-wp-tests.sh in CI #1085

Open
thelovekesh opened this issue Mar 23, 2024 · 5 comments
Open

Replace wp-env with install-wp-tests.sh in CI #1085

thelovekesh opened this issue Mar 23, 2024 · 5 comments

Comments

@thelovekesh
Copy link
Member

thelovekesh commented Mar 23, 2024

Feature Description

While using wp-env is great for local development, we should think carefully about using it for CI testing. Tools like install-wp-tests.sh are built specifically for this purpose and offer several advantages:

  • Faster Tests: install-wp-tests.sh sets up the WordPress environment and runs tests much faster than wp-env.
  • Reliability: We've found issues with wp-env (like in this example: Add standalone PHPUnit configs in plugins/ #1013 (comment) ). Using different tools can help us spot problems more easily among them.
  • Greater Control: install-wp-tests.sh lets us control the testing setup more precisely. This is important if we need to make adjustments for specific scenarios like enabling/disabling particular extensions, enabling coverage extensions, etc.
  • Focused Purpose: install-wp-tests.sh is designed from the ground up for WordPress testing. It does this single task exceptionally well.
  • Faster Fixes: With install-wp-tests.sh, we can solve issues in our CI setup much more quickly. We can always suggest fixes upstream and they can improve our local development but upstream turnaround time should not block CI workflows.
@thelovekesh
Copy link
Member Author

Adding one more point to this: wp-env uses phpunit binary from the vendor directory which causes us to maintain two composer.json (one in root and the other one in build-cs) files to keep compatibility with PHP < 8.1. With install-wp-tests.sh we can set the PHPUnit version globally in CI instead of handling that from vendor/bin binaries.

@joemcgill
Copy link
Member

Thanks @thelovekesh. I don't think that the benefits of maintaining a separate install-wp-tests.sh bash file for installing a test environment for our CI builds is worth the potential downsides.

One thing that I've found to be extremely helpful for developers when working on features is being able to confidently run the same PHPUnit tests (an other tools) locally that will be run by CI. This is particularly true when writing new tests or modifying existing test. Making use of a solution like wp-env abstracts all of the problems with setting up the testing infrastructure which can discourage developers from running and trusting tests that they are creating and running locally.

You're referencing a bash script, install-wp-tests.sh in your description, but it's not clear which version of that bash script you're referring to—perhaps this one maintained in the WP CLI repo? I've seen many different versions of this file copied across many projects over the years (here's all of the versions just in the WordPress org) so having another copy in this repo that we have to maintain ourselves creates an additional burden as new folks join and leave the project.

If we have a need to customize the configuration of the PHP Unit tests in wp-env, we can supply our own WP_TEST_CONFIG_FILE_PATH, which is natively supported by wp-env according to these docs. The advantage here is that the same configuration would be available locally as well as in CI.

I think @felixarntz expressed similar opinions in this PR.

@thelovekesh
Copy link
Member Author

Thanks @joemcgill.

Making use of a solution like wp-env abstracts all of the problems with setting up the testing infrastructure which can discourage developers from running and trusting tests that they are creating and running locally.

But we are only making changes to CI tests to test them more granularly and performantly. It should not affect any developer workflow since wp-env is still there for testing locally. Coming more to test integrity in both environments, I can't remember cases where correct tests will fail in a correctly set up WP environment. Rather it can help us test features more confidently in different environments - IMO correct test cases should pass in each WP test setup regardless of the environment(docker, linux machine etc.) they are being set up.

You're referencing a bash script, install-wp-tests.sh in your description, but it's not clear which version of that bash script you're referring to—perhaps this one maintained in the WP CLI repo? I've seen many different versions of this file copied across many projects over the years (here's all of the versions just in the WordPress org) so having another copy in this repo that we have to maintain ourselves creates an additional burden as new folks join and leave the project.

Humm sorry for the confusion here, I have updated the comment. Here I was referring to one maintained in https://github.com/wp-cli/scaffold-command/blob/main/templates/install-wp-tests.sh but we can fine-tune this per our use case. I just mentioned it to give an idea about using core files from svn + local MySQL setup.

If we have a need to customize the configuration of the PHP Unit tests in wp-env, we can supply our own WP_TEST_CONFIG_FILE_PATH, which is natively supported by wp-env according to these docs. The advantage here is that the same configuration would be available locally as well as in CI.

That's equally true for any tests setup since WP_TEST_CONFIG_FILE_PATH is part of the WP tests bootstrap and not wp-env in particular - https://github.com/WordPress/wordpress-develop/blob/e6da8849a236361b0cfe32604d28b6550c2bffe4/tests/phpunit/includes/bootstrap.php#L6-L7

@joemcgill
Copy link
Member

But we are only making changes to CI tests to test them more granularly and performantly. It should not affect any developer workflow since wp-env is still there for testing locally.

I understand the goal. The scenario that I want to avoid is one where we have made changes to the test configuration in our CI tools that no longer reflect what a developer would experience locally, making it either more difficult or less reliable to write, maintain, and/or run tests locally. I also understand that this is not the intent currently, but it is very easy to have these configurations diverge over time.

For example, we can currently ensure that certain required plugins are installed and activated on the test environment by simply adding them to the .wp-env.json file. Configuring the test environment for folks running tests locally differently from how they are configured in CI can cause disruption that would be good to avoid.

@felixarntz
Copy link
Member

felixarntz commented Apr 1, 2024

@thelovekesh As expressed in previous comments on the other PR, I agree with @joemcgill in that I have concerns about switching away from wp-env as it separates how the test workflows function between CI and local. While my original concern was primarily about ease of setup, it is fair to say that the changes proposed here would only affect CI workflows. But even then this deviation is a problem IMO as it makes it difficult for people to replicate potential problems in CI locally.

Taking a step back here: What I am missing is a concrete list of problems that wp-env has over install-wp-tests.sh. The issue description mentions only broad points, which are hard to reason about. I would like to ask follow up questions like:

  • For which specific aspects that we need does install-wp-tests.sh provide control while wp-env does not?
  • Is there anything that blocks us from contributing those functionalities we need upstream to wp-env?
  • Similar for the reliability aspect, could we address it by contributing upstream to wp-env?
  • How much faster is install-wp-tests.sh compared to wp-env and is this difference worth the switch?

The only point in the issue description which I 100% agree with is that install-wp-tests.sh would allow us to make faster fixes because we're not using a dependency. But IMO that shouldn't be an important consideration, as it applies to using any dependencies. While adding a dependency to a project requires careful consideration, wp-env is well maintained and established in the WordPress project. Instead switching to something we have to fully maintain ourselves does not make sense to me, unless the dependency comes with hard blockers for us where we can't make it do what we want.

Both from a developer experience perspective and a maintenance perspective I am not convinced we should move away from wp-env in CI or locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants