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

GH Actions: actually run the tests on PHP 8.0 + 8.1 #123

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Aug 1, 2021

GH Actions: fail the build if a test run fails

The way things were set up now in the phpunit job, no matter whether tests passed or failed, the workflow would always continue.

I suspect this may have been set-up this way to make sure that all variations of test runs will actually be run ?
The downside is that, while you will see a ❌ for the individual build, the workflow will not be marked as failed if a test runs fails.

I'm proposing to change this now by:

  • Removing the continue-on-error for the test run.
  • Adding the fail-fast key and setting it to false.
    By default this key is set to true, which means that if any individual build within the job fails, all other builds within the job will be cancelled.
    By setting it to false, all builds in the matrix will still be run, but if any of them fail, the workflow will be marked as "failed".

👉🏻 With this setting turned on, you will now see that all builds against PHP 8.0 + 8.1 were actually in a failure state and probably have been since builds against these versions were added.
The PHP 8.1 builds did show as failed, but that was only due to an incompatible Phive version breaking the installation, not because the tests were broken.
Phive has released new version though last week and with that version (0.15.0, automatically installed via setup-php), the build would actually run, but the tests would fail on the same error as the test runs on PHP 8.0.,


Phive: upgrade used version of PHPUnit

The tests were not actually running on PHP 8.0, nor PHP 8.1, due to PHPUnit 8.4 being used, which is not compatible with PHP 8.0+.
The first compatible version in the 8.x range is 8.5 and on the 9.x. range, 9.3.
As for PHP 8.1, there is currently no guarantee yet, but your best bet is using the latest release of PHPUnit.

The push.yml script already contains code to downgrade the PHPUnit version used by Phive for PHP 7.2 back to PHPUnit 8, so it should be safe to upgrade the PHPUnit version for PHP 7.3 and above to PHPUnit 9.5.x.

The only concession which needs to be made is for the phpunit-with-coverage to be run on PHP 7.3 (or to also downgrade to PHPUnit 8).

PHPUnit config: fix configuration

The tests/integration directory does not exist and PHPUnit 9.x will fail on the configuration being invalid.

jrfnl added 3 commits August 1, 2021 21:56
The way things were set up now in the `phpunit` job, no matter whether tests passed or failed, the workflow would always continue.

I suspect this may have been set-up this way to make sure that all variations of test runs will actually be run ?
The downside is that, while you will see a ❌ for the individual build, the workflow will not be marked as failed if a test runs fails.

I'm proposing to change this now by:
* Removing the `continue-on-error` for the test run.
* Adding the `fail-fast` key and setting it to `false`.
    By default this key is set to `true`, which means that if any individual build within the job fails, all other builds within the job will be cancelled.
    By setting it to `false`, all builds in the matrix will still be run, but if any of them fail, the workflow will be marked as "failed".
The tests were not actually running on PHP 8, nor PHP 8.1, due to PHPUnit 8.4 being used, which is not compatible with PHP 8.0+.
The first compatible version in the 8.x range is 8.5 and on the 9.x. range, 9.3.
As for PHP 8.1, there is currently no guarantee yet, but your best bet is using the latest release of PHPUnit.

The `push.yml` script already contains code to _downgrade_ the PHPUnit version used by Phive for PHP 7.2 back to PHPUnit 8, so it should be safe to upgrade the PHPUnit version for PHP 7.3 and above to PHPUnit 9.5.x.

The only concession which needs to be made is for the `phpunit-with-coverage` to be run on PHP 7.3 (or to also downgrade to PHPUnit 8).
The `tests/integration` directory does not exist and PHPUnit 9.x will fail on the configuration being invalid.
@jaapio jaapio merged commit c86c54c into phpDocumentor:1.x Aug 6, 2021
@jrfnl jrfnl deleted the feature/ghactions-actually-run-tests-php-8.0-plus branch August 6, 2021 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants