Skip to content

Commit

Permalink
CI: switch to GitHub Actions - step 5: PHP unit tests
Browse files Browse the repository at this point in the history
This commit:
* Adds a GH Actions workflow for the PHPUnit CI check.
* Removes all references to that check from the `.travis.yml` configuration.

Notes:
1. Builds will run on:
    - Select pushes using branch filtering similar to before.
    - All pull requests.
    - When manually triggered.
        Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration.
    This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown.
    Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off.
    Refs:
    * shivammathur/setup-php#450
    * shivammathur/setup-php#469
4. While the `setup-php` action offers the ability to [install the PHAR file for PHPUnit](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as we need to do a `composer install` anyway to get the other dependencies, like WP Test Utils.
5. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
6. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow.
    The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory.
    The second time, it sets up the PHP version on which we want to do the actual linting.
    Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time)
7. For these BrainMonkey based unit tests, we can (and should) use the most appropriate PHPUnit/BrainMonkey/Mockery/etc version for the PHP version the tests are being run on. Most notably, this is actually needed to get the tests running on PHP 8.0 and higher.
    As there is a committed `composer.lock` file and the PHPUnit version is locked at PHPUnit 5.x, this means we have to do an on-the-fly update of the PHPUnit version depending on the PHP version of the current build.
    As the `composer.lock` file for this plugin also contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the testing to work (on all supported PHP versions).
    Previously in Travis, an uncontrolled `composer update` would be run to get round this.
    Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail.
    So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible.
    - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory and create the generated files for the dependency injection, as well as set up all dependencies.
    - Next, the dependencies which are only needed for the prefixing are removed.
        This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code.
    - Lastly, we remove the `vendor` directory.
    - Now those incompatible dependencies are removed, we switch to the actual PHP version we want to use for the tests.
    - Next, we remove the `phpunit/phpunit` root requirement as it is already a dependency of the WP Test Utils and we don't want to abide by the locked version of PHPUnit.
    - We then run a selective `composer update` to install the locked dependencies and update just and only the test dependencies to match the PHP version on which we will be running the  tests.
    Altogether, this prevents the uncontrolled dependency update.
    Additionally, the `--no-scripts` addition to the Composer run commands when run on the _real_ PHP version, will prevent the prefixing/dependency injection tasks from being run again on incompatible PHP versions.
    Note: no need for adding `--no-interaction` to the plain `composer ...` commands as the `setup-php` action already sets an environment variable to ensure Composer will always run with `--no-interaction` in these workflows.

Differences with the Travis implementation:
* The most appropriate version of the test dependencies will now be used for _all_ builds, not just for builds against PHP 8.0/`nightly`.
* In Travis, there was a reference to a `atanamo/php-codeshift` package, which was removed after the prefixing was finished.
    This package is not a (root) dependency of the WPSEO Free plugin, so removing it doesn't actually have any effect.
    Instead, the `league/oauth2-client` package is removed, as that package should not be used during the test run as it will be prefixed and the prefixed version should be used instead.
* Except for the test dependencies, no other dependencies will be updated during the test run.
* In addition to the PHP versions against which the tests were previously run on Travis, the tests will now also be run against PHP 8.1.
    As PHP 8.1 has been released, the (remaining) test failures on this build need to be fixed, though as these are only _deprecation notices_, the fact that these tests are currently failing is nothing to be too concerned about for the time being.
    To prevent new failures from being introduced in the mean time, I'm not allowing the build to fail, but have instead added expectations for the deprecations to the few tests which still have them.
  • Loading branch information
jrfnl committed Mar 12, 2022
1 parent 253a3bf commit 67e5bd0
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 16 deletions.
78 changes: 78 additions & 0 deletions .github/workflows/unittest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
name: Unit Test

on:
# Run on pushes to select branches and on all pull requests.
push:
branches:
- master
- trunk
- 'release/**'
- 'hotfix/[0-9]+.[0-9]+*'
- 'feature/**'
pull_request:
# Allow manually triggering the workflow.
workflow_dispatch:

# Cancels all previous workflow runs for the same branch that have not yet completed.
concurrency:
# The concurrency group contains the workflow name and the branch name.
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
unit-test:
runs-on: ubuntu-latest

strategy:
matrix:
php_version: ['5.6', '7.0', '7.2', '7.4', '8.0', '8.1']

name: "Unit Test: PHP ${{ matrix.php_version }}"

steps:
- name: Checkout code
uses: actions/checkout@v3

# The prefix-dependencies task makes use of reflection-based PHP code that only works on PHP > 7.2.
- name: Install PHP 7.x for generating the vendor_prefixed directory and dependency injection
uses: shivammathur/setup-php@v2
with:
php-version: 7.2
coverage: none

- name: Install Composer dependencies, generate vendor_prefixed directory and run dependency injection
uses: ramsey/composer-install@v2

# Remove packages which are not PHP cross-version compatible and only used for the prefixing.
# - humbug/php-scoper is only needed to actually do the prefixing, so won't be shipped anyway.
# - league/oauth2-client and its dependencies *are* the packages being prefixed.
- name: Delete dependencies which are not cross-version compatible
run: composer remove --dev --no-scripts humbug/php-scoper league/oauth2-client

- name: Remove vendor directory
run: rm -rf vendor/*

- name: Install PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php_version }}
ini-values: zend.assertions=1, error_reporting=-1, display_errors=On
coverage: none

- name: "Composer: remove PHPUnit root requirement"
run: composer remove --dev phpunit/phpunit --no-update --no-scripts

# Install dependencies and handle caching in one go.
# - Updates the test utilities (and only those!) to the most appropriate version
# for the PHP version on which the tests will be run.
# @link https://github.com/marketplace/actions/install-composer-dependencies
- name: Install Composer dependencies
uses: ramsey/composer-install@v2
with:
# Force a `composer update` run.
dependency-versions: "highest"
# But make it selective.
composer-options: "yoast/wp-test-utils --with-dependencies --no-scripts"

- name: Run unit tests
run: composer test
16 changes: 0 additions & 16 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -211,22 +211,6 @@ script:
vendor/bin/phpunit -c phpunit-integration.xml.dist
travis_time_finish && travis_fold end "PHP.integration-tests"
fi
- |
if [[ "$PHPUNIT" == "1" && ${TRAVIS_PHP_VERSION:0:1} != "8" && $TRAVIS_PHP_VERSION != "nightly" ]]; then
# PHP < 8
travis_fold start "PHP.tests" && travis_time_start
vendor/bin/phpunit
travis_time_finish && travis_fold end "PHP.tests"
fi
- |
if [[ "$PHPUNIT" == "1" ]] && [[ ${TRAVIS_PHP_VERSION:0:1} == "8" || $TRAVIS_PHP_VERSION == "nightly" ]]; then
# PHP >= 8
travis_fold start "PHP.tests" && travis_time_start
travis_retry composer require --dev phpunit/phpunit:"^9.0" --update-with-dependencies --ignore-platform-reqs --no-interaction &&
travis_retry composer update yoast/wp-test-utils --with-all-dependencies --ignore-platform-reqs --no-interaction &&
vendor/bin/phpunit
travis_time_finish && travis_fold end "PHP.tests"
fi
- |
if [[ "$COVERAGE" == "1" ]]; then
travis_fold start "PHP.coverage" && travis_time_start
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/builders/indexable-post-builder-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,12 @@ public function test_set_indexable_repository() {
* @covers ::build
*/
public function test_build( $postmeta, $expected_result ) {
// These two expectations should be removed once the underlying issue has been resolved.
if (PHP_VERSION_ID >= 80100) {
$this->expectDeprecation();
$this->expectDeprecationMessage('Constant FILTER_SANITIZE_STRING is deprecated');
}

Monkey\Functions\expect( 'get_permalink' )->once()->with( 1 )->andReturn( 'https://permalink' );
Monkey\Functions\expect( 'get_post_custom' )->with( 1 )->andReturn( $postmeta );
Monkey\Functions\expect( 'maybe_unserialize' )->andReturnFirstArg();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ public function set_up() {
* @covers ::is_met
*/
public function test_ajax_elementor_save() {
// These two expectations should be removed once the underlying issue has been resolved.
if (PHP_VERSION_ID >= 80100) {
$this->expectDeprecation();
$this->expectDeprecationMessage('Constant FILTER_SANITIZE_STRING is deprecated');
}

// We are in an Ajax request.
Monkey\Functions\expect( 'wp_doing_ajax' )->andReturn( true );

Expand All @@ -67,6 +73,12 @@ public function test_ajax_elementor_save() {
* @covers ::is_met
*/
public function test_not_post_not_elementor_save() {
// These two expectations should be removed once the underlying issue has been resolved.
if (PHP_VERSION_ID >= 80100) {
$this->expectDeprecation();
$this->expectDeprecationMessage('Constant FILTER_SANITIZE_STRING is deprecated');
}

// We are in an Ajax request.
Monkey\Functions\expect( 'wp_doing_ajax' )->andReturn( true );

Expand Down
6 changes: 6 additions & 0 deletions tests/unit/helpers/input-helper-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ public function set_up() {
* @covers ::filter
*/
public function test_filter() {
// These two expectations should be removed once the underlying issue has been resolved.
if (PHP_VERSION_ID >= 80100) {
$this->expectDeprecation();
$this->expectDeprecationMessage('Constant FILTER_SANITIZE_STRING is deprecated');
}

$this->assertNull( $this->instance->filter( \INPUT_POST, 'bogus', \FILTER_SANITIZE_STRING ) );
}
}
6 changes: 6 additions & 0 deletions tests/unit/main-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ protected function set_up() {
* @covers ::get_container
*/
public function test_surfaces() {
// These two expectations should be removed once the underlying issue has been resolved.
if (PHP_VERSION_ID >= 80100) {
$this->expectDeprecation();
$this->expectDeprecationMessage('Constant FILTER_SANITIZE_STRING is deprecated');
}

$container = $this->instance->get_container();

foreach ( $container->getServiceIds() as $service_id ) {
Expand Down

0 comments on commit 67e5bd0

Please sign in to comment.