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

Add instructions for running PHPUnit with wp-env. #2276

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

Add instructions for running PHPUnit with wp-env. #2276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,37 @@ $ vendor/bin/phpunit

The tests will execute and you'll be presented with a summary.

### Running tests using `wp-env`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


To run PHPUnit from within the included `wp-env` environment, you will need to load the development version of the WooCommerce plugin in your environment.

First, clone the WooCommerce repo, replacing `${WC_VERSION}` with the latest version of the plugin (e.g. 8.6.1):

```bash
$ git clone --depth=1 --branch="${WC_VERSION}" https://github.com/woocommerce/woocommerce.git ../woocommerce/`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

```

Build the woocommerce plugin (see [this document](DEVELOPMENT.md) for additional instructions):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- Go to the plugin directory: `cd ../woocommerce/plugins/woocommerce/`
- Install dependencies: `pnpm install`
- Build the plugin: `pnpm --filter=@woocommerce/plugin-woocommerce build`

Go back to the Google Listings and Ads directory and create a `.wp-env.override.json` file containing the following:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

```json
{
"plugins": [
"../woocommerce/plugins/woocommerce",
"."
]
}
```

Restart your wp-env environment: `npm run wp-env start`

You should now be able to run unit tests locally via the CLI: `npm run test:php`.

To pass additional arguments to PHPUnit, make sure they are preceded by an additional `--`, like `npm run test:php -- --filter my_test_method`.

## E2E Testing

E2E testing uses [wp-env](https://developer.wordpress.org/block-editor/reference-guides/packages/packages-env/) which requires [Docker](https://www.docker.com/).
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
"test:e2e-dev": "npx playwright test --config=tests/e2e/config/playwright.config.js --debug",
"test:js": "wp-scripts test-unit-js --coverage",
"test:js:watch": "npm run test:js -- --watch",
"test:php": "wp-env run tests-wordpress --env-cwd=./wp-content/plugins/$(basename $(pwd)) ./vendor/bin/phpunit",
"test-proxy": "node ./tests/proxy",
"wp-env": "wp-env",
"wp-env:up": "npm run -- wp-env start --update",
Expand Down Expand Up @@ -171,4 +172,4 @@
"node": "^16 || ^18",
"npm": "^8 || ^9"
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Loading