Update the Engineering Guidelines Documentation

[maitreyie-chavan]

Problem Statement

The current Engineering Guidelines are mostly outdated and need to be updated to ensure users are able to contribute to the plugin easily

https://github.com/ampproject/amp-wp/wiki/Engineering-Guidelines

To-Do

Along with updating the contents under the existing sections, let's also include a setup guideline for the below development environments

• Local (formerly known as Local by Flywheel)
• Valet -- (to be written by @dhaval-parekh)

Initial updated draft to be created by @delawski, followed by a review and Valet related updates by @dhaval-parekh

Please feel free to add any new sections to improve the knowledge base further.

[delawski]

As part of this issue, I’m working on making sure that the docker-based local environment bundled with the repo works as expected and allows for a really quick set up for new contributors. It is because I realized that that my setup based on Local is pretty custom and not reliable, especially when it comes to PHP unit testing.

[delawski]

I've faced some issues with having 2 instances of both wordpress and mysql running at the same time so that one can be used for development and the other can be used for tests.

It's interesting that @wordpress/env uses such a setup and everything works okay.

I've even tried replacing the custom docker-compose setup we have in the plugin with wp-env. With almost no changes I've been able to make it work (both dev and tests along with xdebug).

Where issues popped up is phpunit. After adding some tweaks to the bootstrap.php file and custom mappings to .wp-env.json (so that tests/includes, tests/data, and phpunit-wp-config.php are found), I was able to run the unit tests inside the tests container like:

wp-env run tests-cli './wp-content/plugins/amp/vendor/bin/phpunit -c wp-content/plugins/amp/phpunit.xml.dist'

There are limitations to that since, when used as an npm script, there's no way to pass additional arguments to phpunit from the CLI.

The wp-env's built in phpunit service seems to be lacking the context (e.g. WP_CLI used in the bootstrap.php file is not defined).

Phpunit, in general, has been a pain for me when using Local, too. It'd be great to find a way to make it easy to run the tests for anyone joining the project (ideally, the tests would work out-of-the-box).

[ediamin]

There are limitations to that since, when used as an npm script, there's no way to pass additional arguments to phpunit from the CLI.

You can use $npm_config_xyz in your npm script and use that to pass additional args to phpunit using --xyz flag. For example in the package.json

  "scripts": {
      ...
      "phpunit": "wp-env run tests-cli './wp-content/plugins/amp/vendor/bin/phpunit -c wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'"
      ...
  },

Then in console we can use,

npm run phpunit --args=/var/www/html/wp-content/plugins/amp/tests/php/test-amp.php
// or
npm run phpunit --args='/var/www/html/wp-content/plugins/amp/tests/php/test-amp.php --filter=test_amp_is_canonical'

Here is my wp-env setup for amp-wp development.

[delawski]

@ediamin Thanks for the advice and for sharing a link to your setup. I think it'll be very helpful.