Skip to content

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Oct 1, 2024

UtilsTest: fix the test failure

Tests on PHP 8.1 and higher (which are running PHPUnit 10+) are failing with the below message:

An error occurred inside PHPUnit.

Message:  Interface "WpOrg\Requests\Transport" not found
Location: /home/runner/work/wp-cli/wp-cli/tests/mock-requests-transport.php:6

This may be due to the test classes being loaded (to count the number of tests) before the test bootstrap is being run (which includes the autoloaders).

If that "guess" is correct, this patch should fix this.

Tests: apply the same change to other test files

While these tests are not showing errors at this time, it is still best practice to include files in the test set_up_before_class() method, not when the file is being read.

This applies this change to all other test files which were using the anti-pattern.

Tests on PHP 8.1 and higher (which are running PHPUnit 10+) are failing with the below message:
```
An error occurred inside PHPUnit.

Message:  Interface "WpOrg\Requests\Transport" not found
Location: /home/runner/work/wp-cli/wp-cli/tests/mock-requests-transport.php:6
```

This may be due to the test classes being loaded (to count the number of tests) before the test bootstrap is being run (which includes the autoloaders).

If that "guess" is correct, this patch should fix this.
@jrfnl jrfnl requested a review from a team as a code owner October 1, 2024 12:35
While these tests are not showing errors at this time, it is still best practice to include files in the test `set_up_before_class()` method, not when the file is being read.

This applies this change to all other test files which were using the anti-pattern.
@swissspidy
Copy link
Member

swissspidy commented Oct 1, 2024

@jrfnl I found the issue! 🎉

So, these failures on PHP 8.1+ happen only when the config is migrated using composer phpunit -- --migrate-configuration.

For PHPUnit tests we specify two bootstrap files:

  1. bootstrap="tests/bootstrap.php" in the config file
    2../vendor/wp-cli/wp-cli-tests/tests/bootstrap.php file if it exists, passed via --bootstrap:
    https://github.com/wp-cli/wp-cli-tests/blob/bda3b5ba21db21ce9ae495795bdc471421f29641/bin/run-php-unit-tests#L13-L14

--bootstrap takes precedence over the config file. But since we also want to load the file from (1), (2) contains this code to search for such a bootstrap file and includes it if found:

https://github.com/wp-cli/wp-cli-tests/blob/bda3b5ba21db21ce9ae495795bdc471421f29641/tests/bootstrap.php#L34-L52

This regex pattern - '/bootstrap="(?P<bootstrap>.*)"/ - has a flaw in that it is greedy and matches everything until the last double quote.

The problem is, with the migrated configuration, all the attributes in the XML file are on one line, so the regex matched way more than just bootstrap="tests/bootstrap.php"

wp-cli/wp-cli-tests#224 should fix this

@swissspidy
Copy link
Member

Much better now:

2 tests triggered 2 PHP warnings:

1) /home/runner/work/wp-cli/wp-cli/php/WP_CLI/Extractor.php:130
file(./no-such-tar.tar.gz): Failed to open stream: No such file or directory

Triggered by:

* ExtractorTest::test_err_extract_tarball
  /home/runner/work/wp-cli/wp-cli/tests/ExtractorTest.php:146

2) /home/runner/work/wp-cli/wp-cli/php/WP_CLI/Extractor.php:51
file(no-such-zip.zip): Failed to open stream: No such file or directory

Triggered by:

* ExtractorTest::test_err_extract_zip
  /home/runner/work/wp-cli/wp-cli/tests/ExtractorTest.php:216

@swissspidy swissspidy added this to the 2.12.0 milestone Oct 1, 2024
@swissspidy swissspidy merged commit 82ce7d9 into wp-cli:main Oct 1, 2024
37 of 42 checks passed
@jrfnl jrfnl deleted the feature/fix-utils-test branch October 1, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants