-
Notifications
You must be signed in to change notification settings - Fork 266
PHPLIB-1147, PHPLIB-1148: Int64 improvements #1112
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
<?php | ||
|
||
namespace MongoDB\Tests\Comparator; | ||
|
||
use MongoDB\BSON\Int64; | ||
use SebastianBergmann\Comparator\Comparator; | ||
use SebastianBergmann\Comparator\ComparisonFailure; | ||
|
||
use function is_numeric; | ||
use function sprintf; | ||
|
||
class Int64Comparator extends Comparator | ||
{ | ||
public function accepts($expected, $actual) | ||
{ | ||
// Only compare if either value is an Int64 and the other value is numeric | ||
return ($expected instanceof Int64 && $this->isComparable($actual)) | ||
|| ($actual instanceof Int64 && $this->isComparable($expected)); | ||
} | ||
|
||
public function assertEquals($expected, $actual, $delta = 0.0, $canonicalize = false, $ignoreCase = false): void | ||
{ | ||
if ($expected == $actual) { | ||
return; | ||
} | ||
|
||
throw new ComparisonFailure( | ||
$expected, | ||
$actual, | ||
'', | ||
'', | ||
false, | ||
sprintf( | ||
'Failed asserting that %s matches expected %s.', | ||
$this->exporter->export($actual), | ||
$this->exporter->export($expected) | ||
) | ||
); | ||
} | ||
|
||
private function isComparable($value): bool | ||
{ | ||
return $value instanceof Int64 || is_numeric($value); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,211 @@ | ||
<?php | ||
|
||
namespace MongoDB\Tests\Comparator; | ||
|
||
use Generator; | ||
use MongoDB\BSON\Int64; | ||
use PHPUnit\Framework\TestCase; | ||
use SebastianBergmann\Comparator\ComparisonFailure; | ||
|
||
class Int64ComparatorTest extends TestCase | ||
{ | ||
/** @dataProvider provideAcceptsValues */ | ||
public function testAccepts(bool $expectedResult, $expectedValue, $actualValue): void | ||
{ | ||
$this->assertSame($expectedResult, (new Int64Comparator())->accepts($expectedValue, $actualValue)); | ||
} | ||
|
||
public static function provideAcceptsValues(): Generator | ||
{ | ||
yield 'Expects Int64, Actual Int64' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => new Int64(123), | ||
'actualValue' => new Int64(123), | ||
]; | ||
|
||
yield 'Expects Int64, Actual int' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => new Int64(123), | ||
'actualValue' => 123, | ||
]; | ||
|
||
yield 'Expects Int64, Actual int string' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => new Int64(123), | ||
'actualValue' => '123', | ||
]; | ||
|
||
yield 'Expects Int64, Actual float' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => new Int64(123), | ||
'actualValue' => 123.0, | ||
]; | ||
|
||
yield 'Expects Int64, Actual float string' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => new Int64(123), | ||
'actualValue' => '123.0', | ||
]; | ||
|
||
yield 'Expects Int64, Actual non-numeric string' => [ | ||
'expectedResult' => false, | ||
'expectedValue' => new Int64(123), | ||
'actualValue' => 'foo', | ||
]; | ||
|
||
yield 'Expects int, Actual Int64' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => 123, | ||
'actualValue' => new Int64(123), | ||
]; | ||
|
||
yield 'Expects int string, Actual Int64' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => '123', | ||
'actualValue' => new Int64(123), | ||
]; | ||
|
||
yield 'Expects float, Actual Int64' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => 123.0, | ||
'actualValue' => new Int64(123), | ||
]; | ||
|
||
yield 'Expects float string, Actual Int64' => [ | ||
'expectedResult' => true, | ||
'expectedValue' => '123.0', | ||
'actualValue' => new Int64(123), | ||
]; | ||
|
||
yield 'Expects non-numeric string, Actual Int64' => [ | ||
'expectedResult' => false, | ||
'expectedValue' => 'foo', | ||
'actualValue' => new Int64(123), | ||
]; | ||
|
||
yield 'Expects float, Actual float' => [ | ||
'expectedResult' => false, | ||
'expectedValue' => 123.0, | ||
'actualValue' => 123.0, | ||
]; | ||
|
||
yield 'Expects numeric string, Actual numeric string' => [ | ||
'expectedResult' => false, | ||
'expectedValue' => '123', | ||
'actualValue' => '123', | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider provideMatchingAssertions | ||
* @doesNotPerformAssertions | ||
*/ | ||
public function testMatchingAssertions($expected, $actual): void | ||
{ | ||
(new Int64Comparator())->assertEquals($expected, $actual); | ||
} | ||
|
||
public static function provideMatchingAssertions(): Generator | ||
{ | ||
yield 'Expected Int64, Actual Int64' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => new Int64(8589934592), | ||
]; | ||
|
||
yield 'Expected Int64, Actual int' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => 8589934592, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize we don't test PHPLIB on 32-bit platforms, but is this integer literal problematic or would PHP interpret it as a float? |
||
]; | ||
|
||
yield 'Expected Int64, Actual int string' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => '8589934592', | ||
]; | ||
|
||
yield 'Expected Int64, Actual float' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => 8589934592.0, | ||
]; | ||
|
||
yield 'Expected Int64, Actual float string' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => '8589934592.0', | ||
]; | ||
|
||
yield 'Expected int, Actual Int64' => [ | ||
'expected' => 8589934592, | ||
'actual' => new Int64(8589934592), | ||
]; | ||
|
||
yield 'Expected int string, Actual Int64' => [ | ||
'expected' => '8589934592', | ||
'actual' => new Int64(8589934592), | ||
]; | ||
|
||
yield 'Expected float, Actual Int64' => [ | ||
'expected' => 8589934592.0, | ||
'actual' => new Int64(8589934592), | ||
]; | ||
|
||
yield 'Expected float string, Actual Int64' => [ | ||
'expected' => '8589934592.0', | ||
'actual' => new Int64(8589934592), | ||
]; | ||
} | ||
|
||
/** @dataProvider provideFailingValues */ | ||
public function testFailingAssertions($expected, $actual): void | ||
{ | ||
$this->expectException(ComparisonFailure::class); | ||
|
||
(new Int64Comparator())->assertEquals($expected, $actual); | ||
} | ||
|
||
public static function provideFailingValues(): Generator | ||
{ | ||
yield 'Expected Int64, Actual Int64' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => new Int64(456), | ||
]; | ||
|
||
yield 'Expected Int64, Actual int' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => 456, | ||
]; | ||
|
||
yield 'Expected Int64, Actual int string' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => '456', | ||
]; | ||
|
||
yield 'Expected Int64, Actual float' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => 8589934592.1, | ||
]; | ||
|
||
yield 'Expected Int64, Actual float string' => [ | ||
'expected' => new Int64(8589934592), | ||
'actual' => '8589934592.1', | ||
]; | ||
|
||
yield 'Expected int, Actual Int64' => [ | ||
'expected' => 8589934592, | ||
'actual' => new Int64(456), | ||
]; | ||
|
||
yield 'Expected int string, Actual Int64' => [ | ||
'expected' => '8589934592', | ||
'actual' => new Int64(456), | ||
]; | ||
|
||
yield 'Expected float, Actual Int64' => [ | ||
'expected' => 8589934592.1, | ||
'actual' => new Int64(456), | ||
]; | ||
|
||
yield 'Expected float string, Actual Int64' => [ | ||
'expected' => '8589934592.1', | ||
'actual' => new Int64(456), | ||
]; | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 class could be in the
src
directory, with namespaceMongoDB\Test\Comparator
. The need to compare Int64 also exist for the developers using this lib.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.
Per our discussion during the team meeting (after this comment was made), we can look into this down the line. A separate ticket could be used to track that.
@alcaeus and I previously discussed publishing various utility functions from our test suite for the benefit of downstream libraries. Various functions for detecting server functionality or comparing BSON documents might prove useful for things like Doctrine and the Laravel integration. I don't believe we have a ticket for that, so I'd suggest grouping all of these suggestions under a single epic to create a collection of shared code downstream test suites. Then the ticket for the Int64 comparator can be grouped under there, along with anything else we consider.
Uh oh!
There was an error while loading. Please reload this page.
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.
Understood, exposing such PHPUnit extension requires that we think about the supported PHPUnit versions compatibility promise. If a user feels the need for this comparator, please raise an issue or add a comment to the new task PHPLIB-1165.