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

Fix npm run test:php errors in wp-env package #7264

Merged
merged 3 commits into from
Sep 22, 2022
Merged

Conversation

thelovekesh
Copy link
Collaborator

@thelovekesh thelovekesh commented Sep 20, 2022

With the update to wp-env 5.0.0, running npm run test:php now results in a fatal error. After @wordpress/env 5.0 you need to define WP_PHPUNIT__TESTS_CONFIG env for WP Tests config file.

See package release notes.

Summary

Fixes #7263

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

After @wordpress/env 5.0 you need to define WP_PHPUNIT__TESTS_CONFIG env for WP Tests config file
@github-actions
Copy link
Contributor

github-actions bot commented Sep 20, 2022

Plugin builds for 1bf9ab3 are ready 🛎️!

@thelovekesh
Copy link
Collaborator Author

Note: Running WordPress/wordpress-develop#trunk tests suite with WordPress 6.0 causes an internal server error. While changing it to WordPress/wordpress-develop#6.0 works fine. Similarly defining core property in wp-env config to "core": "WordPress/WordPress#master", works fine with WordPress/wordpress-develop#trunk.

@westonruter
Copy link
Member

Note: Running WordPress/wordpress-develop#trunk tests suite with WordPress 6.0 causes an internal server error. While changing it to WordPress/wordpress-develop#6.0 works fine. Similarly defining core property in wp-env config to "core": "WordPress/WordPress#master", works fine with WordPress/wordpress-develop#trunk.

OK, so does core then need to be updated in .wp-env.json? As it stands right now, this PR is giving me the following error:

$ npm run test:php

> amp-wp@ test:php /home/westonruter/repos/amp
> wp-env run phpunit 'env WP_PHPUNIT__TESTS_CONFIG=/wordpress-phpunit/wp-tests-config.php WORDPRESS_TABLE_PREFIX=wptests_ WP_TESTS_DIR=/var/www/wordpress-develop/tests/phpunit /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args'

ℹ Starting 'env WP_PHPUNIT__TESTS_CONFIG=/wordpress-phpunit/wp-tests-config.php WORDPRESS_TABLE_PREFIX=wptests_ WP_TESTS_DIR=/var/www/wordpress-develop/tests/phpunit /var/www/html/wp-content/plugins/amp/vendor/bin/phpunit -c /var/www/html/wp-content/plugins/amp/phpunit.xml.dist $npm_config_args' on the phpunit container. 

Starting aabe871bb4b92d5867707c9f76aa7c03_tests-mysql_1 ... done
Starting aabe871bb4b92d5867707c9f76aa7c03_tests-wordpress_1 ... done
Error: wp-tests-config.php is missing! Please use wp-tests-config-sample.php to create a config file.

@thelovekesh
Copy link
Collaborator Author

@westonruter Can you please confirm once if your node_modules are not stale? With the current @wordpress/env: "5.2.0" package I am getting the following results:

image

Confirmed the wp-env version with npm run wp-env -- --version
image

@thelovekesh
Copy link
Collaborator Author

OK, so does core then need to be updated in .wp-env.json? As it stands right now, this PR is giving me the following error:

By default, wp-env considers the latest WP release and its matching test suite from the trunk and its compatible PHPUnit version which in the case right now is 6.0. We are currently providing custom tests dir which is from wordpress-develop.

My thoughts are:

  • Either keep the core and tests from master and trunk branches.
  • Or let them be default as resolved by the package itself.

In the wiki/docs, we can refer the user to wp-env docs so they can provide custom core/test versions according to their needs.

@westonruter
Copy link
Member

I just did the following:

  1. npm install
  2. npm run wp-env destroy
  3. npm run start
  4. npm run test:php

And I get the same internal server error I got above.

When I run npm run wp-env -- --version I also get 5.2.0.

So I don't know what's going on. I'll look a bit deeper.

In terms of testing, we should default to the latest stable version of WordPress. Testing against trunk (i.e. nightly) will be unstable.

@westonruter
Copy link
Member

Strange. The error I'm getting is:

Failed opening required '/var/www/html//wp-includes/class-wpdb.php

Indeed, there is no class-wpdb.php file in ~/.wp-env/aabe871bb4b92d5867707c9f76aa7c03/tests-WordPress/wp-includes. The file is instead wp-db.php.

There seems to be a version mismatch problem here. It seems tests-WordPress is using WordPress 6.0.2, whereas wordpress-develop is running 6.1-beta1-54282-src.

Compare these two lines:

https://github.com/WordPress/wordpress-develop/blob/0e924d96aaff3ad5000f9c9f2aa4998419d75284/tests/phpunit/includes/install.php#L43
https://github.com/WordPress/wordpress-develop/blob/6.0.2/tests/phpunit/includes/install.php#L43

The change seems to have come from WordPress/wordpress-develop@fdb6e13 as part of WPCORE-56268. So the problem is that the test suite PHP includes is coming from 6.1-beta1-54282-src which is trying to load wp-db.php which does not exist in WordPress 6.0.2.

If the version is made consistent then this problem would be fixed.

@thelovekesh
Copy link
Collaborator Author

@westonruter Exactly. Have you tried changing the wordpress-develop mapping to WordPress/wordpress-develop#6.0? It will work fine.

Or, with the trunk tests specify "core": "WordPress/WordPress#master", it should work perfectly.

… version

By default keep the stable version and matching test suite version. Anyone can use a .wp-env.override.json file to override current configs
@thelovekesh
Copy link
Collaborator Author

thelovekesh commented Sep 22, 2022

@westonruter My idea is to keep the latest stable WP version in our config. Later if anyone wants to run tests on another version, they can define a .wp-env.override.json and specify their configs.

After 84162d0 I tried running tests with WP 5.6 in .wp-env.override.json and everything works fine.

{
  "core": "WordPress/WordPress#5.6"
}

@westonruter
Copy link
Member

With the latest changes, it's working now. Nevertheless, it still doesn't seem all the versions are in sync. Inside of my npm run wp-env install-path, I'm seeing:

$ ack 'wp_version =' $(find . -name version.php)
WordPress/wp-includes/version.php
19:$wp_version = '6.0';

wordpress-develop/src/wp-includes/version.php
19:$wp_version = '6.0.3-alpha-54035-src';

tests-WordPress/wp-includes/version.php
19:$wp_version = '6.0';

My idea is to keep the latest stable WP version in our config.

The problem is that it is installing WP 6.0 but the current stable version is actually 6.0.2.

I don't see a way to reference the latest stable version.

Can the three copies of WP all be set to be trunk?

@thelovekesh
Copy link
Collaborator Author

Can the three copies of WP all be set to be trunk?

Yes, it's possible. In that case we need to specify the core as WordPress/WordPress#master and wordpress-develop mapping as WordPress/wordpress-develop#trunk.

With this combination, running ack 'wp_version =' $(find . -name version.php) at installed path gives:

wordpress-develop/src/wp-includes/version.php
19:$wp_version = '6.1-beta1-54282-src';

WordPress/wp-includes/version.php
19:$wp_version = '6.1-beta1-54285';

tests-WordPress/wp-includes/version.php
19:$wp_version = '6.1-beta1-54285';

I am a little confused about the wp_version in wordpress-develop though, not sure how things get updated on the git side from trac.

Try changes at 1bf9ab3.


Thanks for the ack trick. Looks cool.

@westonruter
Copy link
Member

Excellent. I'm seeing:

WordPress/wp-includes/version.php
19:$wp_version = '6.1-beta1-54288';

wordpress-develop/src/wp-includes/version.php
19:$wp_version = '6.1-beta1-54282-src';

tests-WordPress/wp-includes/version.php
19:$wp_version = '6.1-beta1-54288';

I can see that r54288 is WordPress/wordpress-develop@ac8ccca which is the current HEAD of trunk on GitHub.

It is indeed strange about r54282. Not sure why it is 6 revisions behind.

Anyway, it's working much better now so I'm merging. Thanks!

@westonruter westonruter added this to the v2.3.1 milestone Sep 22, 2022
@westonruter westonruter merged commit 56b4050 into develop Sep 22, 2022
@westonruter westonruter deleted the fix/wp-env branch September 22, 2022 18:55
@westonruter
Copy link
Member

It is indeed strange about r54282. Not sure why it is 6 revisions behind.

These are differing revisions:

image

Perhaps one of the copies of WP is using a nightly build pegged at the most recent beta release?

@thelovekesh
Copy link
Collaborator Author

I think trac and git trunk are not in sync. Probably they got synced nightly? I will check it tomorrow, to check if both are in sync.

@westonruter westonruter modified the milestones: v2.3.1, v2.4 Jan 26, 2023
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running npm run test:php is broken after wp-env update to wp-env 5.0.0
2 participants