Make WordPress Core

Opened 8 months ago

Closed 8 months ago

#59394 closed defect (bug) (fixed)

Fix failing unit tests

Reported by: desrosj's profile desrosj Owned by: costdev's profile costdev
Milestone: 6.4 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch has-unit-tests commit fixed-major
Focuses: Cc:

Description (last modified by mukesh27)

PHPUnit x.y.z was released earlier today with a change that addresses a breaking change upstream from PHP.

This causes two failures in the Core test suite:

There were 2 failures:

1) Tests_Admin_WpAutomaticUpdater::test_is_allowed_dir_should_return_true_if_open_basedir_is_set_and_path_is_allowed
Test was run in child process and ended unexpectedly

phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

2) Tests_Admin_WpAutomaticUpdater::test_is_allowed_dir_should_return_false_if_open_basedir_is_set_and_path_is_not_allowed
Test was run in child process and ended unexpectedly

phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:97
/var/www/vendor/bin/phpunit:118

Three versions of PHPUnit were released. Any changes made to fix these failing tests will need to be backported to any branch that uses:

Props @mukesh27 for the original report.

Change History (12)

This ticket was mentioned in Slack in #core by desrosj. View the logs.


8 months ago

#2 @mukesh27
8 months ago

  • Description modified (diff)

This ticket was mentioned in PR #5249 on WordPress/wordpress-develop by @costdev.


8 months ago
#3

  • Keywords has-patch has-unit-tests added; needs-patch removed

In PHPUnit 9.6.13, the child processes used for process isolation now use temporary files to communicate their result to the parent process. This caused a failure in some tests that set the open_basedir PHP directive.

This adds sys_get_temp_dir() to the open_basedir value set by the tests.

References:

#4 @costdev
8 months ago

The failing tests were introduced in [55425] / #42619, so backports should only need to go as far as the 6.2 branch.

#5 @costdev
8 months ago

  • Owner set to costdev
  • Status changed from new to assigned

#6 @costdev
8 months ago

  • Keywords commit added

I tested this on Linux and @SergeyBiryukov tested this on Windows. Tests were successful.

PR 5249 has been reviewed and approved.

Adding for commit and prepping this now.

#7 @costdev
8 months ago

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

In 56622:

Build/Test Tools: Add sys_get_temp_dir() to open_basedir tests.

In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process.

This caused a failure in some tests that set the open_basedir PHP directive to a value that did not include sys_get_temp_dir().

This adds sys_get_temp_dir() to the open_basedir value set by the tests to ensure that permission is still granted for the temporary directory.

PHPUnit uses sys_get_temp_dir(). To ensure the result is the same, Core's get_temp_dir() function is not used.

References:

Props desrosj, mukesh27, SergeyBiryukov, costdev.
Fixes #59394.

@costdev commented on PR #5249:


8 months ago
#8

Committed in r56622.

#9 @costdev
8 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport to the 6.2 and 6.3 branches.

#10 @costdev
8 months ago

In 56624:

Build/Test Tools: Add sys_get_temp_dir() to open_basedir tests.

In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process.

This caused a failure in some tests that set the open_basedir PHP directive to a value that did not include sys_get_temp_dir().

This adds sys_get_temp_dir() to the open_basedir value set by the tests to ensure that permission is still granted for the temporary directory.

PHPUnit uses sys_get_temp_dir(). To ensure the result is the same, Core's get_temp_dir() function is not used.

References:

Props desrosj, mukesh27, SergeyBiryukov, costdev.
Merges [56622] to the 6.3 branch.
See #59394.

#11 @costdev
8 months ago

In 56625:

Build/Test Tools: Add sys_get_temp_dir() to open_basedir tests.

In PHPUnit 10.3.5, 9.6.13 and 8.5.34, the child processes used for process isolation now use temporary files to communicate their result to the parent process.

This caused a failure in some tests that set the open_basedir PHP directive to a value that did not include sys_get_temp_dir().

This adds sys_get_temp_dir() to the open_basedir value set by the tests to ensure that permission is still granted for the temporary directory.

PHPUnit uses sys_get_temp_dir(). To ensure the result is the same, Core's get_temp_dir() function is not used.

References:

Props desrosj, mukesh27, SergeyBiryukov, costdev.
Merges [56622] to the 6.2 branch.
See #59394.

#12 @costdev
8 months ago

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

Backports to the 6.2 and 6.3 branches are complete. Closing the ticket. Thanks everyone!

Note: See TracTickets for help on using tickets.