PHPLIB-1502: Test with PHP 8.4 #1484
Conversation
| @@ -325,9 +325,9 @@ functions: | |||
| working_dir: "src" | |||
| script: | | |||
| ${PREPARE_SHELL} | |||
| file="${PROJECT_DIRECTORY}/.evergreen/install-composer.sh" | |||
| # Don't use ${file} syntax here because evergreen treats it as an empty expansion. | |||
| [ -f "$file" ] && DEPENDENCIES=${DEPENDENCIES} bash $file || echo "$file not available, skipping" | |||
There was a problem hiding this comment.
This line was responsible for Evergreen not exiting with a setup failure when something went wrong. With this change, any error while installing dependencies will cause a setup failure.
| $latestPhpVersion = max($supportedPhpVersions); | ||
| // TODO: use max() once PHP 8.4 is stable | ||
| //$latestPhpVersion = max($supportedPhpVersions); | ||
| $latestPhpVersion = '8.3'; |
There was a problem hiding this comment.
Since PHP 8.4 isn't stable yet, I figured that the extra tests that run on the latest PHP version should continue to run on 8.3
| @@ -38,4 +38,9 @@ php --ri mongodb | |||
|
|
|||
| install_composer | |||
|
|
|||
| # Remove psalm as it's not compatible with PHP 8.4: https:/vimeo/psalm/pull/10928 | |||
| if [ "$PHP_VERSION" == "8.4" ]; then | |||
| php composer.phar remove --no-update --dev vimeo/psalm | |||
There was a problem hiding this comment.
Since we only run PHPUnit, I decided to explicitly remove the psalm dependency instead of ignoring any upper bounds for PHP version constraints, as that may result in actually incompatible packages being pulled in.
| @@ -860,6 +860,9 @@ public function testDanglingOpenWritableStream(): void | |||
| $code = <<<'PHP' | |||
| require '%s'; | |||
| require '%s'; | |||
| // Don't report deprecations - if the issue exists this code will | |||
| // result in a fatal error | |||
| error_reporting(E_ALL & ~E_DEPRECATED & ~E_USER_DEPRECATED); | |||
There was a problem hiding this comment.
Was this due to disableMD5 being used below? Is that even relevant to what's being tested here? Why not remove it and allow deprecation notices to fail the test?
There was a problem hiding this comment.
The deprecation in question is about implicit nullable args in a PHPUnit class, which is loaded when we call createTestClient as it's in the inheritance chain. The issue is that checking for no output is the only way to test this behaviour. It goes back to #779 which fixed PHPLIB-345 - without the fix PHP fails with an autoloading failure as it already started unloading things when open resources (including streams with custom stream wrappers) are closed, leading to an autoload failure that doesn't find a class.
| if (version_compare(phpversion(), '8.4', '>=')) { | ||
| $this->markTestIncomplete('Test fails on PHP 8.4 due to deprecations'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Nit: I prefer using the PHP_VERSION_ID for performance reason.
| if (version_compare(phpversion(), '8.4', '>=')) { | |
| $this->markTestIncomplete('Test fails on PHP 8.4 due to deprecations'); | |
| } | |
| if (PHP_VERSION_ID >= 8_04_00) { | |
| $this->markTestIncomplete('Test fails on PHP 8.4 due to deprecations'); | |
| } | |
But the best is to use the #[RequiresPhp('>= 8.4')] attribute.
There was a problem hiding this comment.
Ah, good point. There's also #[RequiresPhpExtension] that we can use in another tests that depends on the extension version. I wonder if we can also leverage this mechanism for server version checks - I'll look into it.
There was a problem hiding this comment.
Might be tricky for server version checks as it'd need to use a persistent client connection. But there may be an opportunity there to streamline everything with a service that captures all of that information when PHPUnit is first loaded (fetching the URI from environment variables).
PHPLIB-1502
This adds testing with PHP 8.4 on evergreen. Due to an implicit nullable parameter in PHPUnit, I had to skip a single test. This test can be re-enabled when we merge this up to v1.x.