WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 5 months ago

Last modified 7 weeks ago

#41123 closed defect (bug) (fixed)

Modified `WP_TESTS_TITLE` causes dirty wp-api-generated.js

Reported by: boonebgorges Owned by: jnylen0
Milestone: 4.9 Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: has-patch commit
Focuses: Cc:

Description

A value of WP_TESTS_TITLE other than the default 'Test Blog' causes wp-api-generated.js to be modified during PHPUnit test runs. This leaves me with a dirty index, which leaves me feeling dirty. Is it possible for the test suite to clean up after itself, and/or to make the changes to an untracked file?

Attachments (3)

41123.diff (2.6 KB) - added by jnylen0 5 months ago.
compare-wp-api-fixtures.js (1.7 KB) - added by jnylen0 5 months ago.
41123.2.diff (12.0 KB) - added by jnylen0 5 months ago.

Download all attachments as: .zip

Change History (9)

@jnylen0
5 months ago

#1 @jnylen0
5 months ago

When a change is made to the REST API there should be some indication that this fixture file needs to be updated as well. Currently that shows up as the file being regenerated with changes and marked as dirty. It could also be a failing test or something like that - this setup has gotten a bit complicated and could use some re-working.

41123.diff should fix the issue for now. We need to add some lines to this code block which protects parts of the fixture file against changes that can happen in between PHPUnit runs. Here are the steps originally used to generate it.

Should we also guard against changes to WP_TESTS_DOMAIN and WP_TESTS_EMAIL? In particular, changing WP_TESTS_DOMAIN causes a lot more changes to the fixture file.

#2 @boonebgorges
5 months ago

The approach in 41123.diff seems good to me, though I defer to those who set up the wp-api-generated.js tests :)

Should we also guard against changes to WP_TESTS_DOMAIN and WP_TESTS_EMAIL? In particular, changing WP_TESTS_DOMAIN causes a lot more changes to the fixture file.

This is a good idea!

@jnylen0
5 months ago

#3 @jnylen0
5 months ago

  • Owner set to jnylen0
  • Status changed from new to accepted

Here is how to use compare-wp-api-fixtures.js to generate a patch like 41123.2.diff:

  1. In tests/phpunit/tests/rest-api/rest-schema-setup.php, set private static $fixture_replacements to an empty array.
  1. In wp-tests-config, set the WP_TESTS_* constants to their default values:
define( 'WP_TESTS_DOMAIN', 'example.org' );
define( 'WP_TESTS_EMAIL', 'admin@example.org' );
define( 'WP_TESTS_TITLE', 'Test Blog' );
  1. Run these commands:
phpunit --filter WP_Test_REST_Schema_Initialization
cp tests/qunit/fixtures/wp-api-generated.js wp-api-generated-1class.js
  1. Set the WP_TESTS_* constants to something other than the default values. For example:
define( 'WP_TESTS_DOMAIN', 'not-example.org' );
define( 'WP_TESTS_EMAIL', 'admin@not-example.org' );
define( 'WP_TESTS_TITLE', 'Super Test Blog' );
  1. Run these commands:
phpunit --filter REST
cp tests/qunit/fixtures/wp-api-generated.js wp-api-generated-REST.js
  1. Download compare-wp-api-fixtures.js from this ticket to your WP install.
  1. Run these commands:
npm install deep-diff@0.3.4
node compare-wp-api-fixtures.js
  1. Copy the stdout result of the Node.js script into the $fixture_replacements array.

#4 @jnylen0
5 months ago

  • Keywords has-patch commit added
  • Milestone changed from Awaiting Review to 4.9

#5 @jnylen0
5 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 41006:

Ensure that wp-api.js test fixtures do not change with WP_TESTS_* constants.

Previously, changing these constants in wp-tests-config would cause PHPUnit to regenerate wp-api-generated.js with different values.

This commit uses the existing mechanism to also "freeze" all values that would change as a result of changing these constants.

Fixes #41123.

This ticket was mentioned in Slack in #core-committers by boone. View the logs.


7 weeks ago

Note: See TracTickets for help on using tickets.