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

Tests: make the testsuite compatible with PHPUnit 9 and test against PHP 8 #278

Closed
wants to merge 5 commits into from

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Oct 4, 2020

Summary

This PR can be summarized in the following changelog entry:

  • Preparation for compatibility with PHP 8

Relevant technical choices:

Composer: update the dependencies of the test dependencies

Update the composer.lock file with the latest dependencies related to the test suite.

composer update --dev brain/monkey phpunit/phpunit --with-dependencies

Note: the update is still based on platform: php 5.6, but some of the more recent sub-dependencies have widened their PHP version constraints, which makes getting the tests running on PHP 8 easier.

Tests: use annotations instead of fixtures

In PHPUnit 8.x, the setUpBeforeClass(), tearDownAfterClass(), setUp() and tearDown() methods have received type declarations,with a return type void, making it neigh impossible to implement this in a cross-version manner as the void return type is not available until PHP 7.1.

However, PHPUnit also supports @beforeClass, @afterClass, @before and @after annotations to run arbitrary methods for setting things up/tearing things down. Those annotations have been around forever and are supported all the way back to PHPUnit 4.x.

So instead of doing convoluted things with extending different classes for different PHPUnit versions or creating classes on the fly for this, by renaming the fixture related methods and using the PHPUnit annotations, the unit test suite becomes compatible with PHPUnit 8.x without much effort.

Tests: implement work-around for remaining PHPUnit cross-version issue

PHPUnit 9 removed the assertInternalType() method in favour of more specific methods, which were introduced in PHPUnit 8.

As this test suite only uses this assertion in one test file, a simple helper method with an if/else does the trick.

Composer: allow installation of PHPUnit 8 and 9

Now the tests have been made cross-version compatible with PHPUnit 8 and 9, allow installation of those versions by widening the version restraints in the composer.json file.

PHPUnit 8 introduces a form of caching to PHPUnit, so adding this cache file to the .gitignore file.

Travis: run the tests against PHP 8/nightly

Now the tests are fully compatible with PHPUnit 9, the test suite can be run on PHP 8.

As not all dependencies of PHPUnit have tagged a new version which allows for PHP 8 yet, we do need to ignore-platform-reqs for the time being.

Includes adding travis_retry to composer install-like commands to get round builds stalling on the connection with Packagist, especially on older PHP versions.

Test instructions

This PR can be tested by following these steps:

  • Verify the output of all builds for this PR, making sure that 1) the installation via Composer works without issues, 2) the tests are being run and 3) the tests pass.

To test locally, it will be simplest to:

  1. Download phars of PHPUnit 7/8/9 and save them somewhere in the system path or in the root directory of this project (an exclude them from Git).
  2. On a recent PHP 7.4.x version, run the tests using each of the downloaded phars:
    phpunit7.phar
    phpunit8.phar
    phpunit9.phar
    
    The tests on each version should run without any warnings and pass. This confirms the PHPUnit cross-version compatibility.
  3. On a recent PHP 8 pre-release, run the tests again, though now only using the phpunit9.phar.
    The tests should pass, which confirms PHP 8 compatibility for the code covered by tests.

Update the `composer.lock` file with the latest dependencies related to the test suite.
```
composer update --dev brain/monkey phpunit/phpunit --with-dependencies
```

Note: the update is still based on `platform: php 5.6`, but some of the more recent sub-dependencies have widened their PHP version constraints, which makes getting the tests running on PHP 8 easier.
In PHPUnit 8.x, the `setUpBeforeClass()`, `tearDownAfterClass()`, `setUp()` and `tearDown()` methods have received type declarations,with a return type `void`, making it neigh impossible to implement this in a cross-version manner as the `void` return type is not available until PHP 7.1.

However, PHPUnit also supports `@beforeClass`, `@afterClass`, `@before` and `@after` annotations to run arbitrary methods for setting things up/tearing things down. Those annotations have been around forever and are supported all the way back to PHPUnit 4.x.

So instead of doing convoluted things with extending different classes for different PHPUnit versions or creating classes on the fly for this, by renaming the fixture related methods and using the PHPUnit annotations, the unit test suite becomes compatible with PHPUnit 8.x without much effort.
PHPUnit 9 removed the `assertInternalType()` method in favour of more specific methods, which were introduced in PHPUnit 8.

As this test suite only uses this assertion in one test file, a simple helper method with an if/else does the trick.
@jrfnl jrfnl added yoast cs/qa changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog labels Oct 4, 2020
@jrfnl jrfnl added this to the 3.x / Next Release milestone Oct 4, 2020
Now the tests have been made cross-version compatible with PHPUnit 8 and 9, allow installation of those versions by widening the version restraints in the `composer.json` file.

PHPUnit 8 introduces a form of caching to PHPUnit, so adding this cache file to the `.gitignore` file.
@jrfnl jrfnl force-pushed the JRF/tests-cross-version-compat branch from 54fc910 to 4c05178 Compare October 4, 2020 16:42
Now the tests are fully compatible with PHPUnit 9, the test suite can be run on PHP 8.

As not all dependencies of PHPUnit have tagged a new version which allows for PHP 8 yet, we do need to `ignore-platform-reqs` for the time being.

Includes adding `travis_retry` to `composer install`-like commands to get round builds stalling on the connection with Packagist, especially on older PHP versions.
@jrfnl jrfnl added the blocked label Oct 29, 2020
@jrfnl
Copy link
Contributor Author

jrfnl commented Oct 29, 2020

I'm closing this PR for now as it needs to be updated to use the PHPUnit Polyfills and WP Test Tests. Will re-open once updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog yoast cs/qa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant