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

assertSame broken after 9.5.14 version #5045

Closed
pablorsk opened this issue Sep 13, 2022 · 9 comments
Closed

assertSame broken after 9.5.14 version #5045

pablorsk opened this issue Sep 13, 2022 · 9 comments
Labels
feature/assertion Issues related to assertions and expectations type/bug Something is broken

Comments

@pablorsk
Copy link

pablorsk commented Sep 13, 2022

Q A
PHPUnit version 9.5.14
PHP version 8.1.10
Installation Method Composer

Summary

Assuming the following test:

public function testAssertSame()
{
    static::assertSame(40.7, 40.8-0.1);
}

In version 9.5.13 this test works perfect:

.                                                                   1 / 1 (100%)

But, in version 9.5.14, it breaks

1) SomeTest::testAssertSame
Failed asserting that 40.699999999999996 is identical to 40.7.

Any ideas? I try to set serialize_precision = 16, but the problem continued.

@pablorsk pablorsk added the type/bug Something is broken label Sep 13, 2022
@sebastianbergmann
Copy link
Owner

CC @marijnvanwezel @mvorisek

@sebastianbergmann sebastianbergmann added the feature/assertion Issues related to assertions and expectations label Sep 14, 2022
@sebastianbergmann
Copy link
Owner

This may be fixed by #4972.

@mvorisek
Copy link
Contributor

assertSame pass only if the values are strictly the same, in your case, the values are not the same, you need to compare them with a small delta using assertEqualsWithDelta

@bancer
Copy link

bancer commented Dec 9, 2022

@sebastianbergmann #4972 did not fix the problem:

Screenshot from 2022-12-09 16-39-16

@mvorisek
Copy link
Contributor

mvorisek commented Dec 9, 2022

@bancer the assertion is correctly not met - https://3v4l.org/5CiYa

@bancer
Copy link

bancer commented Dec 12, 2022

@bancer the assertion is correctly not met - https://3v4l.org/5CiYa

Well, everybody could assume that 40.7 is mathematically the same value as 40.8-0.1.

@mvorisek
Copy link
Contributor

mvorisek commented Dec 12, 2022

you can use $delta in assertEquals, assertSame checks (simply said) for 1:1 binary value

@anders-photowall
Copy link

anders-photowall commented Mar 3, 2023

I ran in to this problem today after doing what is supposed to be a safe composer update since we have caret notation and minor and patch updates should never have breaking changes. This very much is a breaking change and not even on a minor but patch version which I find extremely surprising. Was this an unintended and unknown breaking change? Or should the phpunit lib be considered unsafe and potentially breaking to update even patch changes on? That should be very clearly stated in the documentation in that case.

@marijnvanwezel
Copy link
Contributor

I think that strictly speaking, the change could be considered a patch only. The previous behaviour (<= 9.5.13) was incorrect, as an assertion could pass even if it clearly shouldn't have. Comparisons between floats should not be done with assertSame, but it might be good to more clearly mention this in the documentation.

That being said, since many people relied on this incorrect behaviour, including it in a major release might have been a better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/assertion Issues related to assertions and expectations type/bug Something is broken
Projects
None yet
Development

No branches or pull requests

6 participants