phpunit: assertEquals unexpectedly fails on equivalent strings with decimals ('0', '0.00')

We are also affected by this heavy bc break. While I agree that the edge cases in #58 are bad and should be avoided, I don’t think changing assertEquals for this is the correct move. AssertEquals has always been (to me), about loose comparison that potentially includes type juggling. If you know both values are of type string and you want to make sure, the above edge cases do not trigger like '0.0' !== '0', just use assertSame. Basically assertEquals is now a mix of assertEquals and assertSame depending on the types which makes the behavior not obvious at all.

Eventually: yes. This will take time (for thinking and discussing).

yep, it’s not. maybe let us introduce assertEqualsObjects that requires to pass 2 objects of same type, and deprecate assertEquals to discourage (and later forbid) relying on type casting ?

Is the contract for assertEquals that if php treats them as equal according to ==, that the assertion should pass? And if you want strict comparison with ===, to use assertSame instead?

indeed, at least according to docs 😉

no it is not @IanVS , change of mine was targetting 7.2. 7.1.4 brought that regression, 7.1.3 works

Both examples are failind due to changes in comparator packages, as a workaround add "sebastian/comparator": "^2.1", to your composer file, and it will work for you.

On the meantime, I will dig into comparator 3 issue

These are strings: they are to be compared as strings. == is used internally when the types do not match.

@sebastianbergmann For my own knowledge and so that everyone is clear, what is the “expected” result of assertions like $this->assertEquals('+1', '1'); and $this->assertEquals('0', '0.00');?

The assertion would fail. The correct assertions you can use there are:

• $self::assertEquals(+1, '1');
• $self::assertEquals(+1, 1);
• $self::assertEquals(0, '0.00');
• $self::assertEquals('0', 0.00);

If both are strings, type juggling is only dangerous, as you may be passing the incorrect type of data from one layer to the next one, and testing would not even catch those.

ah, between I open the page, disappear for meeting, come back and type my answer - new input is here. just saw 😄

I did a little more investigation, and this started to fail for me on 7.1.5, perhaps due to a major-version upgrade of sebastian/comparator in that version. In fact, I’m now guessing the behavior I’m seeing is due to https://github.com/sebastianbergmann/comparator/pull/58.

And to be clear, I’m not saying this is necessarily a bad change. In fact, it may have found a few bugs in my code that were passing tests before. It was just an unexpected breaking change in a patch version bump.