-
Notifications
You must be signed in to change notification settings - Fork 265
PHPLIB-1369 Upgrade to PHPUnit 10 #1412
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
Conversation
4cde98f
to
83bd963
Compare
@@ -69,12 +70,15 @@ public function setUp(): void | |||
$this->configuredFailPoints = []; | |||
} | |||
|
|||
public function tearDown(): void | |||
protected function onNotSuccessfulTest(Throwable $t): never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I like this way of handling failed tests 💪
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Available since PHPUnit 3.4 (sebastianbergmann/phpunit@8e6c10c), yet there's no trace of this in the documentation 🤷
@@ -77,7 +77,6 @@ jobs: | |||
php-ini-values: "zend.assertions=1" | |||
|
|||
- name: "Run PHPUnit" | |||
run: "vendor/bin/phpunit -v" | |||
run: "vendor/bin/phpunit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about removing the output of skipped tests, so I agree with this change 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also, the -v
option isn't available on PHPUnit 10
a453911
to
2f9bed3
Compare
996967e
to
08e1870
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question about the Rector config, but everything else LGTM.
@@ -16,13 +18,17 @@ | |||
]); | |||
|
|||
// Modernize code | |||
$rectorConfig->sets([LevelSetList::UP_TO_PHP_74]); | |||
$rectorConfig->sets([ | |||
LevelSetList::UP_TO_PHP_74, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this have been bumped to PHP 8.1 in 24214b6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PHP 8.1 rules have to be tuned. Some like ReadOnlyPropertyRector and ClassPropertyAssignToConstructorPromotionRector need to be challenged.
https://jira.mongodb.org/browse/PHPLIB-1536
@@ -24,16 +25,17 @@ public function assertEquals($expected, $actual, $delta = 0.0, $canonicalize = f | |||
return; | |||
} | |||
|
|||
$exporter = new Exporter(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted that the internal property was removed from the Comparator base class in sebastianbergmann/comparator@bf22fc1
@@ -69,12 +70,15 @@ public function setUp(): void | |||
$this->configuredFailPoints = []; | |||
} | |||
|
|||
public function tearDown(): void | |||
protected function onNotSuccessfulTest(Throwable $t): never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Available since PHPUnit 3.4 (sebastianbergmann/phpunit@8e6c10c), yet there's no trace of this in the documentation 🤷
public function testModifierDocuments($modifiers, stdClass $expectedSort): void | ||
{ | ||
(new CommandObserver())->observe( | ||
function () use ($modifiers): void { | ||
// @todo revert this lines after PHPC-2457 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, this should become irrelevant after #1439
@@ -3,6 +3,7 @@ | |||
namespace MongoDB\Tests; | |||
|
|||
use MongoDB; | |||
use PHPUnit\Framework\Attributes\DataProvider; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR, but I'm wondering what use MongoDB;
above is used for. Looks like this dates back to eafad05.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prevent duplication of class names as use
statement and in the SKIPPED_CLASSES
list.
@@ -205,7 +208,7 @@ protected function getCollectionName(): string | |||
{ | |||
$class = new ReflectionClass($this); | |||
|
|||
return sprintf('%s.%s', $class->getShortName(), hash('crc32b', $this->getName())); | |||
return sprintf('%s.%s', $class->getShortName(), hash('xxh3', $this->name())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted: https://xxhash.com/ available since PHP 8.1 (see: hash_algos()
).
@@ -256,7 +257,7 @@ private function assertMatchesOperator(BSONDocument $operator, $actual, string $ | |||
$constraint = IsBsonType::anyOf(...(array) $operator['$$type']); | |||
|
|||
if (! $constraint->evaluate($actual, '', true)) { | |||
self::failAt(sprintf('%s is not an expected BSON type: %s', $this->exporter()->shortenedExport($actual), implode(', ', $types)), $keyPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted that $types
was never defined. This dates back to 4608a73. Curious that it wasn't caught by static analysis, unless this snuck into the baseline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* v1.x: (95 commits) PHPLIB-1369 Upgrade to PHPUnit 10 (#1412) Higher phpunit version required (#1463) Fix deprecations in tests (#1458) Deprecate functionality to be removed (#1441) Expect BulkWriteException (#1455) Merge v1.20 into v1.x (#1447) PHPLIB-1525 Removes dependency to Symfony PHPUnit bridge (#1413) Change deprecated assertObjectHasAttribute to assertObjectHasProperty (#1432) Performance: Keep collections and indexes between GridFS tests (#1421) Add final annotations to non-internal Operation classes (#1410) Fix types accepted by $round (#1401) Replace arrayHasKey with assertArrayHasKey in tests (#1403) PHPLIB-1514 Make data providers static (#1404) PHPLIB-1515 Replace assertObjectHasAttribute with assertObjectHasProperty (#1405) Restore Prose22_RangeExplicitEncryptionTest (#1400) Remove Prose22_RangeExplicitEncryptionTest that requires ext-mongodb < 1.20 (#1394) Use `match` instead of `switch` when a simple value is returned (#1393) Remove PHPUnit functions polyfill (#1395) Update branch names for GHA workflows (#1390) PHPLIB-1419 Encode Agg builder objects in Collection methods (#1383) ...
* v1.x: (101 commits) PHPLIB-1541: Include specs repository as a submodule (#1429) PHPLIB-1548 Inherit `typeMap` option in `Collection::listSearchIndexes()` (#1482) Fix array shape for `Collection::listSearchIndex($options)` (#1480) PHPLIB-1545: Deprecate CreateCollection flags option and related constants (#1477) Fix junit logging (#1475) Deprecate typeMap on operations without meaningful result (#1473) PHPLIB-1369 Upgrade to PHPUnit 10 (#1412) Higher phpunit version required (#1463) Fix deprecations in tests (#1458) Deprecate functionality to be removed (#1441) Expect BulkWriteException (#1455) Merge v1.20 into v1.x (#1447) PHPLIB-1525 Removes dependency to Symfony PHPUnit bridge (#1413) Change deprecated assertObjectHasAttribute to assertObjectHasProperty (#1432) Performance: Keep collections and indexes between GridFS tests (#1421) Add final annotations to non-internal Operation classes (#1410) Fix types accepted by $round (#1401) Replace arrayHasKey with assertArrayHasKey in tests (#1403) PHPLIB-1514 Make data providers static (#1404) PHPLIB-1515 Replace assertObjectHasAttribute with assertObjectHasProperty (#1405) ...
Fix PHPLIB-1369
@annotations
to#[Attributes]
hasFailed
with the methodonNotSuccessfulTest
inFunctionalTestCase
createStub
ingetInvalidDocumentCodecValues
TestCase::exporter()
is deprecated andComparisonFailure
5th argument was removed)