Skip to content

PHPLIB-1502: Test with PHP 8.4 #1484

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 16, 2024
Merged

PHPLIB-1502: Test with PHP 8.4 #1484

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a46f253
PHPLIB-1502: Test with PHP 8.4
alcaeus Oct 14, 2024
3e82bd1
Remove psalm from dependencies on PHP 8.4
alcaeus Oct 14, 2024
57949f1
Suppress deprecations in tests expecting no output
alcaeus Oct 15, 2024
8b2a763
Skip test failing due to PHPUnit deprecations
alcaeus Oct 15, 2024
865f8c5
Skip failing test on PHP 8.4
alcaeus Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .evergreen/config/functions.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

DEPENDENCIES=${DEPENDENCIES} \
PHP_VERSION=${PHP_VERSION} \
bash ${PROJECT_DIRECTORY}/.evergreen/install-composer.sh

"start load balancer":
- command: shell.exec
Expand Down
5 changes: 4 additions & 1 deletion .evergreen/config/generate-config.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

// Supported PHP versions. Add new versions to the beginning of the list
$modernPhpVersions = [
'8.4',
'8.3',
'8.2',
'8.1',
Expand All @@ -13,7 +14,9 @@
];
$supportedPhpVersions = array_merge($modernPhpVersions, $legacyPhpVersions);

$latestPhpVersion = max($supportedPhpVersions);
// TODO: use max() once PHP 8.4 is stable
//$latestPhpVersion = max($supportedPhpVersions);
$latestPhpVersion = '8.3';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$lowestPhpVersion = min($supportedPhpVersions);

// Supported MongoDB versions. Add new versions after "rapid"
Expand Down
10 changes: 10 additions & 0 deletions .evergreen/config/generated/build/build-extension-next-minor.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions .evergreen/config/generated/build/build-extension.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 61 additions & 0 deletions .evergreen/config/generated/test-variant/modern-php-full.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions .evergreen/install-composer.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@ php --ri mongodb

install_composer

# Remove psalm as it's not compatible with PHP 8.4: https://github.com/vimeo/psalm/pull/10928
if [ "$PHP_VERSION" == "8.4" ]; then
php composer.phar remove --no-update --dev vimeo/psalm
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

fi

php composer.phar update $COMPOSER_FLAGS
6 changes: 6 additions & 0 deletions tests/GridFS/BucketFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$client = MongoDB\Tests\FunctionalTestCase::createTestClient();
$database = $client->selectDatabase(getenv('MONGODB_DATABASE') ?: 'phplib_test');
$gridfs = $database->selectGridFSBucket();
Expand Down Expand Up @@ -903,6 +906,9 @@ public function testDanglingOpenWritableStreamWithGlobalStreamWrapperAlias(): vo
$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);
$client = MongoDB\Tests\FunctionalTestCase::createTestClient();
$database = $client->selectDatabase(getenv('MONGODB_DATABASE') ?: 'phplib_test');
$database->selectGridFSBucket()->registerGlobalStreamWrapperAlias('alias');
Expand Down
6 changes: 6 additions & 0 deletions tests/Model/CodecCursorFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
use MongoDB\Model\CodecCursor;
use MongoDB\Tests\FunctionalTestCase;

use function phpversion;
use function restore_error_handler;
use function set_error_handler;
use function version_compare;

use const E_DEPRECATED;
use const E_USER_DEPRECATED;
Expand All @@ -25,6 +27,10 @@ public function setUp(): void

public function testSetTypeMap(): void
{
if (version_compare(phpversion(), '8.4', '>=')) {
$this->markTestIncomplete('Test fails on PHP 8.4 due to deprecations');
}

$collection = self::createTestClient()->selectCollection($this->getDatabaseName(), $this->getCollectionName());
$cursor = $collection->find();

Expand Down
6 changes: 6 additions & 0 deletions tests/UnifiedSpecTests/Constraint/MatchesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
use stdClass;

use function hex2bin;
use function phpversion;
use function preg_quote;
use function version_compare;

class MatchesTest extends FunctionalTestCase
{
Expand Down Expand Up @@ -69,6 +71,10 @@ public function testOperatorExists(): void

public function testOperatorType(): void
{
if (version_compare(phpversion(), '8.4', '>=')) {
$this->markTestIncomplete('Test fails on PHP 8.4 due to deprecations');
}

Comment on lines +74 to +77
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I prefer using the PHP_VERSION_ID for performance reason.

Suggested change
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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

$c = new Matches(['x' => ['$$type' => 'string']]);
$this->assertResult(true, $c, ['x' => 'foo'], 'string matches string type');
$this->assertResult(false, $c, ['x' => 1], 'integer does not match string type');
Expand Down