-
Notifications
You must be signed in to change notification settings - Fork 384
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
Update repo local environment setup #6802
Conversation
package.json
Outdated
"test:js": "wp-scripts test-unit-js --config=tests/js/jest.config.js", | ||
"test:js:help": "wp-scripts test-unit-js --help", | ||
"test:js:watch": "npm run test:js -- --watch", | ||
"test:php": "vendor/bin/phpunit", | ||
"test:php:help": "npm run test:php -- --help", | ||
"test:php": "wp-env run phpunit 'WORDPRESS_TABLE_PREFIX=wptests_ /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest using the tests-wordpress
container instead of the phpunit
, because there is no xdebug support in the phpunit container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used Gutenberg setup as a base for this:
Do you think we need xdebug when running phpunit tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not always necessary. But having xdebug during TDD is very helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xdebug is needed for code coverage, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that xdebug will be enabled on all environments since we're start it using wp-env start --xdebug
.
I got an error on my local when I tried one of the following:
- "test:php": "wp-env run phpunit ..."
+ "test:php": "wp-env run tests-wordpress ..."
# or
- "test:php": "wp-env run phpunit ..."
+ "test:php": "wp-env run tests-cli ..."
@ediamin Is there a way to check if xdebug is enabled with the test:php
script as it is currently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use env VAR_NAME=var_value
to solve this problem in the script,
"test:php": "wp-env run tests-wordpress 'env WORDPRESS_TABLE_PREFIX=wptests_ WP_PHPUNIT__TESTS_CONFIG=/var/www/html/phpunit-wp-config.php /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ediamin This works well for changing the table prefix however now I'm getting some xdebug issues:
[12-Jan-2022 14:14:14 UTC] Xdebug: [Step Debug] Could not connect to debugging client. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(
[12-Jan-2022 14:14:14 UTC] Xdebug: [Step Debug] Could not connect to debugging client. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(
Installing...
[12-Jan-2022 14:14:15 UTC] Xdebug: [Step Debug] Could not connect to debugging client. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(
[12-Jan-2022 14:14:15 UTC] Xdebug: [Step Debug] Could not connect to debugging client. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(
[12-Jan-2022 14:14:15 UTC] Xdebug: [Step Debug] Could not connect to debugging client. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-(
Despite the notices, the test suites are executed as expected.
How about adding a new task for running phpunit with xdebug like:
"test:php": "wp-env run phpunit 'WORDPRESS_TABLE_PREFIX=wptests_ /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", | |
"test:php": "wp-env run phpunit 'WORDPRESS_TABLE_PREFIX=wptests_ /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", | |
"test:php:xdebug": "wp-env run tests-wordpress 'env WORDPRESS_TABLE_PREFIX=wptests_ WP_PHPUNIT__TESTS_CONFIG=/var/www/html/phpunit-wp-config.php /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can have a separate task for that. Actually if you start debugging or start listening for the xdebug in your IDE/editor then those message will be gone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, thanks! After I turned on listening for xdebug connections, I got no errors and debugging within phpunit tests started to work as expected.
I think we shouldn't expect every contributor to turn on xdebug before running the tests. That's why having a separate script for running phpunit with xdebug support makes even more sense (added in 2beedcb).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've incorporated this into GoogleChromeLabs/pwa-wp#778
After reverting and using I asked @ediamin and @dhaval-parekh for testing out this branch(i.e. the new local environment). The E2E tests were passing for them just as they do for me. Neither of us were using I don't know why is it not working on CI. I guess we'll have to just keep the |
I've filed this as WordPress/gutenberg#41038. I tried patching the package locally and I got further, but not there yet. Some changes I've stashed: diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php
index 4b2cd2176..b16868687 100644
--- a/includes/sanitizers/class-amp-form-sanitizer.php
+++ b/includes/sanitizers/class-amp-form-sanitizer.php
@@ -219,7 +219,13 @@ protected function get_action_url( $action_url ) {
*/
if ( ! $action_url ) {
// phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotValidated
- return esc_url_raw( '//' . $_SERVER['HTTP_HOST'] . wp_unslash( $_SERVER['REQUEST_URI'] ) );
+ $action_url = '//' . $_SERVER['HTTP_HOST'];
+ if ( isset( $_SERVER['SERVER_PORT'] ) && ! in_array( (int) $_SERVER['SERVER_PORT'], [ 80, 443 ], true ) ) {
+ $action_url .= ':' . (int) $_SERVER['SERVER_PORT'];
+ }
+ $action_url .= wp_unslash( $_SERVER['REQUEST_URI'] );
+
+ return esc_url_raw( $action_url );
}
$parsed_url = wp_parse_url( $action_url );
diff --git a/tests/php/src/TestCase.php b/tests/php/src/TestCase.php
index d4e275292..fe802210d 100644
--- a/tests/php/src/TestCase.php
+++ b/tests/php/src/TestCase.php
@@ -27,6 +27,13 @@ public function set_up() {
// This was needed with the upgrade of yoast/wp-test-utils from 0.2.2 to 1.0.0.
$wp_the_query = $wp_query;
+
+ // Ensure the proper environment to work around <https://github.com/WordPress/gutenberg/issues/41038>.
+ $parsed_home = wp_parse_url( home_url() );
+ $_SERVER['HTTP_HOST'] = $parsed_home['host'];
+ if ( ! empty( $parsed_home['port'] ) ) {
+ $_SERVER['SERVER_PORT'] = $parsed_home['port'];
+ }
}
/** |
…98-local-env * 'develop' of github.com:ampproject/amp-wp: (26 commits) Update TikTok test fixture Expand on description for native HTML image tag toggle Keep deletion toggle at end of section Apply suggestion from PR Apply suggestion from PR Update variable type Update script to generate sanitizers/class-amp-allowed-tags-generated.php Fix PHPstan issue after enabling treatPhpDocTypesAsCertain option Update comment for type declaration Update SupportData class Check instance type before calling function Raise phpstan level from 3 to 4 and fixed errors Bump @babel/core from 7.17.10 to 7.17.12 (#7103) Bump @actions/github from 5.0.2 to 5.0.3 (#7101) Bump google/cloud-storage from 1.26.3 to 1.27.0 (#7102) Bump @babel/plugin-proposal-class-properties from 7.16.7 to 7.17.12 (#7104) Bump eslint-plugin-jest from 26.1.5 to 26.2.2 (#7105) Bump actions/setup-node from 3.1.1 to 3.2.0 (#7106) Merge pull request #7099 from ampproject/dependabot/npm_and_yarn/actions/github-5.0.2 Change the text of toggle button that switch to native img ...
Tests now pass in Docker! However, you'll have to patch your local copy of
|
The last thing I haven't yet been able to solve is getting PhpStorm's debugger to properly engage when accessing the development instance via the browser. I think this is due to the |
If I change the server name to However, it's not using the path mappings I have defined for It's creating another duplicate server with the manual path mapping I selected: And if I try to set a breakpoint for a file in the AMP repo, then it's not thereafter prompting me to manually set a mapping. It shows an error in the debugger instead: So it seems we do need to configure it specifically to reuse the existing server somehow. |
It seems the problem is that I configured the server with the host name of And with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is finally ready for use! Thank you @delawski for laying the foundation here. This will greatly simplify contributor onboarding in the future.
Wow! So great to see it has finally landed in |
How did you get this to work with different PHP * WP versions and such? We tried wp-env for our repo a few times (GoogleForCreators/web-stories-wp#5612) but there were still too many blockers:
|
Well, it is only suitable for whatever the default PHP version which is captures in WordPress/gutenberg#40745. It also requires patching wp-env to fix an issue with My issues not yet closed:
So yeah, I should say that your mileage may vary at the moment! But it has been usable for me so far for this project. |
Summary
Fixes #6798
Because of a long and mostly irrelevant commit history on this branch, I suggest squashing the commits before merging.
The existing local environment shipped with the plugin is based on a custom Docker setup. Even though we recommend using it in the Engineering Guidelines, none of the main contributors seem to be using it.
There are various external reasons for this, e.g. personal preferences or working on other projects alongside AMP (e.g. WordPress Core). There are also internal factors that discourage developers from using the bundled
local-env
. It's not easy to set up and it doesn't provide PHPUnit and E2E tests support out of the box.This PR attempts to solve these problems by replacing the
local-env
with the@wordpress/env
package.Below you'll find the proposed update to the Engineering Guidelines wiki page.
Getting started
Requirements
To contribute to this plugin, you need the following tools installed on your computer:
Local environment
Since you need a WordPress environment to run the plugin, the quickest way to get up and running is to use the provided
wp-env
environment.Prerequisites
wp-env
requires Docker to be installed. There are instructions available for installing Docker on Windows 10 Pro, all other versions of Windows, macOS, and Linux.Installation
You can then clone this project somewhere on your computer and
cd
to the directory:git clone git@github.com:ampproject/amp-wp.git amp cd amp
Next, install the required version of Node using NVM:
Then, install the required Javascript and PHP dependencies:
Finally, build the assets:
Usage
Starting the environment
In order to start the local environment, go to the plugin root directory and run the following command (note that Docker needs to be running beforehand):
wp-env
will set up 2 independent environments for you:development
available at http://localhost:8888 (Username:admin
, Password:password
)tests
available at http://localhost:8889 (Username:admin
, Password:password
)They both share the plugin files but use separate databases. Thanks to that, changes introduced by PHPUnit or E2E tests will not affect the
development
environment.Stopping the environment
If you wish to stop the local environment, use the following command:
Note that the database will not be cleared when starting or stopping the environment. If you would like to clear the database you can run the following:
Resetting and destroying
You can also destroy Docker containers and volumes and start over by using:
Using
wp-env
CLI directlyIf you would like to use one of the
wp-env
CLI commands directly and you don't havewp-env
installed globally, make sure to run the command via the npm script like:Using WP-CLI
You can run WP CLI commands using the
wp
script like:The commands will be run in the development environment. To run a WP CLI command in the tests container, use the
wp-env
command directly:More information
Please, refer to the
@wordpress/env
README for more information on how to use, extend, and troubleshootwp-env
.Developing the plugin
Whether you use the pre-existing local environment or a custom one, any PHP code changes will be directly visible during development.
However, for Javascript this involves a build process. To watch for any Javascript file changes and re-build it when needed, you can run the following command:
This way you will get a development build of the Javascript, which makes debugging easier.
To get a production build, run:
Branches
The branching strategy follows the GitFlow schema; make sure to familiarize yourself with it.
All branches are named with with the following pattern:
{type}
/{issue_id}
-{short_description}
{type}
= issue Type label{issue_id}
= issue ID{short_description}
= short description of the PRTo include your changes in the next patch release (e.g.
2.0.x
), please base your branch off of the current release branch (e.g.2.0
) and open your pull request back to that branch. If you open your pull request with thedevelop
branch then it will be by default included in the next minor version (e.g.2.x
).Code reviews
All submissions, including submissions by project members, require review. We use GitHub pull requests for this purpose. Consult GitHub Help for more information on using pull requests.
Coding standards
All contributions to this project will be checked against WordPress-Coding-Standards with PHPCS, and for JavaScript linting is done with ESLint.
To verify your code meets the requirements, you can run
npm run lint
.You can also install a
pre-commit
hook via:Or if running inside Lando:
This way, your code will be checked automatically before committing any changes.
Todo: Opt to use Husky instead.
Xdebug
By default Xdebug is started when running
npm run env:start
. Read more about Xdebug in wp-env.VS Code
See Gutenberg's Xdebug IDE Support docs.
PhpStorm/IntelliJ Configuration
When running Xdebug with PhpStorm, the
PHP_IDE_CONFIG
environment variable needs to be set. Runningnpm run test:php:xdebug
will automatically includePHP_IDE_CONFIG="serverName=amp"
. This allows you to create a server with the name oflocalhost
, and also usinglocalhost
for the host and providing port8888
:Path mappings should map
/var/www/html/wp-content/plugins/amp
to the repo on your filesystem.Then under Include path, map your install path (found via
npm run wp-env install-path
) to/var/www/wordpress-develop
, and then thesrc
subdirectory to/var/www/html
.You'll want to make sure that this Install path is also included in your PHP Include Path:
The PHP Debug preferences may be configured as follows (which should be the default):
You now should be able to set a breakpoint and PhpStorm's debugger should pause once you've started listening for PHP Debug Connections via the telephone icon:
The debugger should be invoked both via web connections to
http://localhost:8888
as well as PHPUnit runs vianpm run -s test:php:xdebug
(see below).Tests
PHP Unit Tests
The AMP plugin uses the PHPUnit testing framework to write unit and integration tests for the PHP part. PHPUnit is run in the tests instance provided by
wp-env
.To run the full test suite, you can use the following command:
Tip: Add the
--silent
/-s
argument to reduce the noise when running commands via npm.You can also just run test for a specific function or class by using something like this:
npm run -s test:php --args="--filter=AMP_Theme_Support"
Note that you should pass any additional arguments to PHPUnit via the
--args
as presented in the example above.To run PHPUnit tests with Xdebug running:
See
npm run test:php:help
to see all the possible options.JavaScript Unit Tests
Jest is used as the JavaScript unit testing framework.
To run the full test suite, you can use the following command:
You can also watch for any file changes and only run tests that failed or have been modified:
See
npm run test:js:help
to get a list of additional options that can be passed to the test runner.End-to-End Tests
This project leverages the local
wp-env
tests instance to facilitate end-to-end (E2E) testing using Puppeteer. Note that E2E and PHPUnit tests use different databases. This way they won't interfere with each other.To run the full test suite, you can use the following command:
You can also watch for any file changes and only run tests that failed or have been modified:
Want to run a specific test? Use the Jest CLI options to filter the test suites to be run:
npm run test:e2e -- -t "Template Mode selector on AMP Settings screen"
For debugging purposes, you can also run the E2E tests in non-headless mode:
Note that this will also slow down all interactions during tests by 80ms. You can control these values individually too:
Sometimes one might want to test additional scenarios that aren't possible in a WordPress installation out of the box. That's why the test setup allows for for adding some utility plugins that can be activated during E2E tests.
For example, such a plugin could create a custom post type and the accompanying E2E test would verify that block validation errors are shown for this custom post type too.
These plugins can be added to
tests/e2e/plugins
and then activated as part of the E2E test.Testing media and embed support
The following script creates a post in order to test support for WordPress media and embeds:
After running the script, go to the URL that is output in the command line.
Testing widgets support
The following script adds an instance of every default WordPress widget to the first registered sidebar:
After running the script, there will be a message indicating which sidebar has the widgets. Please visit a page with that sidebar.
Testing comments support
The following script creates a post with comments in order to test support for WordPress comments:
After running the script, go to the URL that is output in the command line.
Testing Gutenberg block support
The following script creates a post with all core Gutenberg blocks:
After running the script, go to the URL that is output in the command line.
Updating allowed tags and attributes
The file
class-amp-allowed-tags-generated.php
has the AMP specification's allowed tags and attributes. It's used in sanitization.To update that file, perform the following steps:
cd
to the root of this plugin./bin/amphtml-update.sh
(orlando ssh -c './bin/amphtml-update.sh'
if using Lando).This script is intended for a Linux environment like VVV or Lando wordpressdev.
Creating a plugin build
To create a build of the plugin for installing in WordPress as a ZIP package, run:
This will create an
amp.zip
in the plugin directory which you can install. The contents of this ZIP are also located in thebuild
directory which you canrsync
somewhere as well if needed.You can also build the plugin in development mode (for unminified scripts and styles):
local-env
setup with@wordpress/env
Checklist