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

Fix float comparison precision #102

Merged
merged 1 commit into from Sep 14, 2022
Merged

Fix float comparison precision #102

merged 1 commit into from Sep 14, 2022

Conversation

mvorisek
Copy link
Contributor

needed for sebastianbergmann/phpunit#4972

fixed EPSILON was problematic and small values like 1E-100 and 1E-200 were considered the same

this PR removes the DoubleComparator completely, if number is supposed to be compared unexactly, delta can be passed to NumericComparator

.gitignore Outdated Show resolved Hide resolved
@sebastianbergmann sebastianbergmann merged commit 967a104 into sebastianbergmann:3.0 Sep 14, 2022
@sebastianbergmann
Copy link
Owner

The following test failures in PHPUnit 8.5's test suite now occur:

1) PHPUnit\Framework\AssertTest::testAssertEqualsSucceeds with data set #19 ('0.33333333333333', 0.33333333333333337)
Failed asserting that 0.33333333333333337 matches expected '0.33333333333333'.

/usr/local/src/phpunit/src/Framework/Constraint/IsEqual.php:100
/usr/local/src/phpunit/src/Framework/Assert.php:2952
/usr/local/src/phpunit/src/Framework/Assert.php:635
/usr/local/src/phpunit/tests/unit/Framework/AssertTest.php:266
/usr/local/src/phpunit/src/Framework/TestCase.php:1492
/usr/local/src/phpunit/src/Framework/TestCase.php:1112
/usr/local/src/phpunit/src/Framework/TestResult.php:703
/usr/local/src/phpunit/src/Framework/TestCase.php:838
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/TextUI/TestRunner.php:659
/usr/local/src/phpunit/src/TextUI/Command.php:235
/usr/local/src/phpunit/src/TextUI/Command.php:194

2) PHPUnit\Framework\AssertTest::testAssertEqualsSucceeds with data set #20 (0.3333333333333333, '0.33333333333333')
Failed asserting that '0.33333333333333' matches expected 0.3333333333333333.

/usr/local/src/phpunit/src/Framework/Constraint/IsEqual.php:100
/usr/local/src/phpunit/src/Framework/Assert.php:2952
/usr/local/src/phpunit/src/Framework/Assert.php:635
/usr/local/src/phpunit/tests/unit/Framework/AssertTest.php:266
/usr/local/src/phpunit/src/Framework/TestCase.php:1492
/usr/local/src/phpunit/src/Framework/TestCase.php:1112
/usr/local/src/phpunit/src/Framework/TestResult.php:703
/usr/local/src/phpunit/src/Framework/TestCase.php:838
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/TextUI/TestRunner.php:659
/usr/local/src/phpunit/src/TextUI/Command.php:235
/usr/local/src/phpunit/src/TextUI/Command.php:194

3) PHPUnit\Framework\AssertTest::testAssertEqualsSucceeds with data set #27 (0.3333333333333333, 0.33333333333333337)
Failed asserting that 0.33333333333333337 matches expected 0.3333333333333333.

/usr/local/src/phpunit/src/Framework/Constraint/IsEqual.php:100
/usr/local/src/phpunit/src/Framework/Assert.php:2952
/usr/local/src/phpunit/src/Framework/Assert.php:635
/usr/local/src/phpunit/tests/unit/Framework/AssertTest.php:266
/usr/local/src/phpunit/src/Framework/TestCase.php:1492
/usr/local/src/phpunit/src/Framework/TestCase.php:1112
/usr/local/src/phpunit/src/Framework/TestResult.php:703
/usr/local/src/phpunit/src/Framework/TestCase.php:838
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/TextUI/TestRunner.php:659
/usr/local/src/phpunit/src/TextUI/Command.php:235
/usr/local/src/phpunit/src/TextUI/Command.php:194

4) PHPUnit\Framework\AssertTest::testAssertNotEqualsFails with data set #19 ('0.33333333333333', 0.33333333333333337)
Failed asserting that exception of type "PHPUnit\Framework\AssertionFailedError" is thrown.

/usr/local/src/phpunit/src/Framework/Constraint/Constraint.php:120
/usr/local/src/phpunit/src/Framework/Constraint/Constraint.php:55
/usr/local/src/phpunit/src/Framework/Assert.php:2952
/usr/local/src/phpunit/src/Framework/TestCase.php:1555
/usr/local/src/phpunit/src/Framework/TestCase.php:1112
/usr/local/src/phpunit/src/Framework/TestResult.php:703
/usr/local/src/phpunit/src/Framework/TestCase.php:838
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/TextUI/TestRunner.php:659
/usr/local/src/phpunit/src/TextUI/Command.php:235
/usr/local/src/phpunit/src/TextUI/Command.php:194

5) PHPUnit\Framework\AssertTest::testAssertNotEqualsFails with data set #20 (0.3333333333333333, '0.33333333333333')
Failed asserting that exception of type "PHPUnit\Framework\AssertionFailedError" is thrown.

/usr/local/src/phpunit/src/Framework/Constraint/Constraint.php:120
/usr/local/src/phpunit/src/Framework/Constraint/Constraint.php:55
/usr/local/src/phpunit/src/Framework/Assert.php:2952
/usr/local/src/phpunit/src/Framework/TestCase.php:1555
/usr/local/src/phpunit/src/Framework/TestCase.php:1112
/usr/local/src/phpunit/src/Framework/TestResult.php:703
/usr/local/src/phpunit/src/Framework/TestCase.php:838
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/TextUI/TestRunner.php:659
/usr/local/src/phpunit/src/TextUI/Command.php:235
/usr/local/src/phpunit/src/TextUI/Command.php:194

6) PHPUnit\Framework\AssertTest::testAssertNotEqualsFails with data set #27 (0.3333333333333333, 0.33333333333333337)
Failed asserting that exception of type "PHPUnit\Framework\AssertionFailedError" is thrown.

/usr/local/src/phpunit/src/Framework/Constraint/Constraint.php:120
/usr/local/src/phpunit/src/Framework/Constraint/Constraint.php:55
/usr/local/src/phpunit/src/Framework/Assert.php:2952
/usr/local/src/phpunit/src/Framework/TestCase.php:1555
/usr/local/src/phpunit/src/Framework/TestCase.php:1112
/usr/local/src/phpunit/src/Framework/TestResult.php:703
/usr/local/src/phpunit/src/Framework/TestCase.php:838
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/Framework/TestSuite.php:619
/usr/local/src/phpunit/src/TextUI/TestRunner.php:659
/usr/local/src/phpunit/src/TextUI/Command.php:235
/usr/local/src/phpunit/src/TextUI/Command.php:194

@mvorisek Can you please send a PR to PHPUnit's 8.5 branch to address this? Thanks!

@olivernybroe
Copy link

This change actually broke multiple of our tests in a minor version.

Wasn't that big a problem to fix, but took quite a while to troubleshoot why our tests suddenly gave errors like these
image

@mvorisek
Copy link
Contributor Author

This change actually broke multiple of our tests in a minor version.

Wasn't that big a problem to fix, but took quite a while to troubleshoot why our tests suddenly gave errors like these image

this PR is continuation of sebastianbergmann/phpunit@136fae3 which effectively implies zero delta for numbers greater than 1 - that is why you hit this problem

also, casting from float to string depends on php.ini precision, in phpunit, this problem is fixed now and the full precision is always displayed - in the output you posted, the Actual: 0.1505 should display the full float precision, please submit an issue to the phpunit printer console you use - see exporter sebastianbergmann/exporter@a9f4590 fix

@olivernybroe
Copy link

@mvorisek Yep wasn't much of work to fix the issues when it became clear why 👍

Also big fan of this change, just wanted to add a comment here for people in the future hitting this.

@bancer
Copy link

bancer commented Sep 22, 2022

This broke our tests too:

Screenshot from 2022-09-22 17-01-13

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.

None yet

4 participants