Add instructions for running PHPUnit with wp-env.

[joemcgill]

Changes proposed in this Pull Request:

This updates the README with the setup instructions for running PHPUnit tests with wp-env. This also adds a new test:php script to the package.json file to simplify running the tests locally.

Detailed test instructions:

1. Try following the instructions to confirm that PHPUnit tests are running locally

Additional details:

Changelog entry

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.0%. Comparing base (28211b8) to head (2e26c85).
Report is 60 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #2276   +/-   ##
=======================================
  Coverage     54.0%   54.0%           
=======================================
  Files          293     293           
  Lines         3715    3715           
  Branches       532     532           
=======================================
  Hits          2006    2006           
  Misses        1258    1258           
  Partials       451     451           

Flag	Coverage Δ
js-unit-tests	54.0% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[mikkamp]

Thanks @joemcgill it's working, and I'm able to run unit tests in wp-env. I just left a few comments on where the instructions can clarify a bit better.

There are also several places in our unit tests where we work specifically with the domain http://example.org which is what the local DB for unit tests sets the site_url to. With wp-env that gets a URL like http://localhost:8889 which results in 5 failing tests.

Do you think it would be good to replace those tests with a URL from site_url instead to make them a bit more flexible? One of them needs to remove the http:// part as well, so we can't replace them one on one.

Also it seems in bootstrap.php we set the constant GLA_TESTS_DATA_DIR to the full path of ./data, since that's not set in the wp-env the test test_view_render_fails_if_context_variable_missing fails. Is that an environment variable we can easily set or do you suggest to alter the test to get the path from a different location?

[mikkamp]

I'm assuming this should link to DEVELOPMENT.md in the WC repo? Can we set that link to the right repo?

[joemcgill]

Yes of course 🙃. I'll update it to the full URL.

[mikkamp]

If this is meant to be an override / local only file, can we get it included in .gitignore so it's not accidentally checked in.

[joemcgill]

That would probably be a good idea. I have .wp-env.override.json in my global .gitignore for that reason. Adding it at the project level would be a nice safeguard.

[mikkamp]

Can we explicitly mention this is for running "unit tests" so it's doesn't get confused with the E2E tests being run with wp-env?

[joemcgill]

👍🏻 Sure! It's already a sub-heading under the PHPUnit section, but making it more clear never hurts.

[mikkamp]

All the instructions assume we are going to place woocommerce in the parent directory of google-listings-and-ads. Commonly that would be in wp-content/plugins/. Considering that we are also placing a copy of the monorepo in that folder and not just the woocommerce plugin, I think this construction could generally lead to a bit of confusion.

On my install if I'd have a development version of WooCommerce installed I'd have a symlink in my wp-content/plugins directory installed. I placed the full directory in .wp-env.override.json so it would point to the right development copy of WooCommerce, but we might need to clarify that in these instructions.

[joemcgill]

Right, this is not as intuitive as it could be. Essentially, I'm trying to avoid adding these files to the same folder that the GLA plugin repo lives, so folks don't have to worry about another .gitignore, which is why we check out the mono-repo in the parent directory of the gla plugin. We could alternatively check it out in a special folder in the root directory where GLA is checked out and .gitignore that folder instead.

The .wp-env.override.json file will only load the development version of the woocoomerce plugin from the monorepo in the plugins folder of the environment and not any of the other plugins from the mono-repo. Really, the only reason this is needed is because the unit tests use some of the test classes from the woocommerce development repo as a dependency. If those helpers could be loaded in another way during the PHPUnit bootstrap process, then this could be a bit simpler.

[mikkamp]

Thanks, I'm fine with leaving it as is (I prefer it outside the plugin folder). Just a note to clarify that any location can be used (since a lot of people would already have a copy or I could point it to the copy installed by bin/install-wp-tests.sh) and where it needs an updated path should be helpful enough.

[mikkamp]

If those helpers could be loaded in another way during the PHPUnit bootstrap process, then this could be a bit simpler.

The bootstrap process doesn't really have a lot of heavy requirements, it just loads 4 files directly with the full path: https://github.com/woocommerce/google-listings-and-ads/blob/2.6.0/tests/bootstrap.php#L63-L66

It also checks for the presence of bootstrap.php but that's only to confirm the legacy path, which can easily be checked differently.

Building the WooCommerce plugin is required because it loads a copy of the plugin during unit testing. However that's not required to be a dev copy. So for unit testing it would just as happily run with the following two (as long as the WC version matched between the two):

• A WooCommerce plugin installed from WordPress.org
• A tests directory with those 4 required test files present.

I wouldn't mind changing bootstrap.php to check in a different location to encounter those test files (check in a different location and otherwise fallback to where it's looking now). That way we'd just need a mapping in wp-env to tell it where the test files are. You'd lose a little flexibility of being able to run unit tests with any WC branch, but would simplify the process.

[mikkamp]

Doesn't seem like we need to change this, as the next person (with it auto enabled) is probably going to add it again.

[joemcgill]

Agreed. I think my editor auto-formatted this during save.