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

[IanVS]

Q	A
PHPUnit version	7.3-g2e9ab30
PHP version	7.1.18
Installation Method	Composer

composer info | sort

antecedent/patchwork 2.1.8 Method redefinition (monkey-... aws/aws-sdk-php 3.10.1 AWS SDK for PHP - Use Amazon... cebe/markdown 1.1.0 A super fast, highly extensi... cocur/slugify v0.7 Converts a string into a slug. composer/ca-bundle dev-master 134242f Lets you find a path to the ... doctrine/annotations dev-master 2497b1f Docblock Annotations Parser doctrine/cache dev-master beb0fa3 Caching library offering an ... doctrine/collections dev-master 42c4039 Collections Abstraction library doctrine/common dev-master a3e240f Common Library for Doctrine ... doctrine/inflector dev-master 40daa06 Common String Manipulations ... doctrine/instantiator dev-master 870a62d A small, lightweight utility... doctrine/lexer dev-master cc709ba Base library for a lexer tha... facebook/fbmock dev-master b90fce5 FBMock is a PHP mocking fram... firebase/php-jwt v5.0.0 A simple library to encode a... friendsofsymfony/rest-bundle 1.8.x-dev 12081a7 This Bundle provides various... giggsey/libphonenumber-for-php 5.9.1 Unofficial PHP Port of Googl... google/apiclient 1.0.5-beta Client library for Google APIs guzzlehttp/guzzle 6.3.0 Guzzle is a PHP HTTP client ... guzzlehttp/promises dev-master 2e3e428 Guzzle promises library guzzlehttp/psr7 dev-master b27e0bd PSR-7 message implementation... hamcrest/hamcrest-php dev-master be5380f This is the PHP port of Hamc... ircmaxell/password-compat 1.0.x-dev 9b99377 A compatibility library for ... jms/metadata 1.6.0 Class/method/property metada... jms/parser-lib dev-master 6067cc6 A library for easily creatin... jms/serializer-bundle 1.5.0 Allows you to easily seriali... jms/serializer dev-master f4683f4 Library for (de-)serializing... league/csv 8.x-dev fa8bc05 Csv data manipulation made e... league/oauth2-client 2.2.1 OAuth 2.0 Client Library maknz/slack 1.7.0 A simple PHP package for sen... mikey179/vfsStream v1.2.0 mockery/mockery 1.0 Mockery is a simple yet flex... monolog/monolog 1.x-dev fd8c787 Sends your logs to files, so... mtdowling/cron-expression v1.0.4 CRON for PHP: Calculate the ... mtdowling/jmespath.php 2.4.0 Declaratively specify how to... myclabs/deep-copy 1.x-dev 3e01bda Create deep copies (clones) ... nutshell/nutshell-billing dev-master 8514786 Billing for Nusthell paragonie/random_compat v2.0.11 PHP 5.x polyfill for random_... phar-io/manifest dev-master 29654cc Component for reading phar.i... phar-io/version 2.0.0 Library for handling version... phpcollection/phpcollection 0.5.0 General-Purpose Collection L... phpdocumentor/reflection-common 1.0.1 Common reflection classes us... phpdocumentor/reflection-docblock 4.3.0 With this component, a libra... phpdocumentor/type-resolver 0.4.0 phpoption/phpoption 1.5.0 Option Type for PHP phpspec/prophecy dev-master 6471ce6 Highly opinionated mocking f... phpunit/dbunit dev-master 22843ee PHPUnit extension for databa... phpunit/php-code-coverage dev-master 8656625 Library that provides collec... phpunit/php-file-iterator dev-master cecbc68 FilterIterator implementatio... phpunit/php-text-template 1.2.1 Simple template engine. phpunit/php-timer dev-master 9ef9968 Utility class for timing phpunit/php-token-stream dev-master 711ca0c Wrapper around PHP's tokeniz... phpunit/phpunit dev-master 2e9ab30 The PHP Unit Testing framework. psr/http-message dev-master f6561bf Common interface for HTTP me... psr/log dev-master 4ebe3a8 Common interface for logging... quickbooks/v3-php-sdk dev-master c69c881 The Official PHP SDK for Qui... recurly/recurly-client dev-master 6f6d82a The PHP client library for t... ruudk/twitter-oauth dev-request-headers 665b1b4 PHP 5.3 version of abraham/t... sabre/vobject 3.5.x-dev 129d805 The VObject library for PHP ... sebastian/code-unit-reverse-lookup dev-master 22f5f5f Looks up which function or m... sebastian/comparator dev-master c9eb293 Provides the functionality t... sebastian/diff dev-master 366541b Diff implementation sebastian/environment dev-master 32c5cba Provides functionality to ha... sebastian/exporter dev-master 2096271 Provides the functionality t... sebastian/global-state dev-master 30367ea Snapshotting of global state sebastian/object-enumerator dev-master 06d95dc Traverses array structures a... sebastian/object-reflector dev-master 7707193 Allows reflection of object ... sebastian/recursion-context dev-master dbe1869 Provides functionality to re... sebastian/resource-operations dev-master 0f9911f Provides a list of PHP built... sebastian/version 2.0.1 Library that helps with mana... segmentio/analytics-php 1.4.2 Segment Analytics PHP Library sensio/distribution-bundle dev-master 51554e1 The base bundle for the Symf... sensio/framework-extra-bundle 2.3.x-dev f9e4c97 This bundle provides a way t... sensio/generator-bundle 2.5.x-dev ab9345f This bundle generates code f... sensiolabs/security-checker 4.1.x-dev d539ccb A security checker for your ... symfony/monolog-bundle 2.x-dev c710da0 Symfony MonologBundle symfony/polyfill-apcu dev-master cec3239 Symfony polyfill backporting... symfony/polyfill-intl-icu dev-master 4aa0b65 Symfony polyfill for intl's ... symfony/polyfill-mbstring dev-master 7c8fae0 Symfony polyfill for the Mbs... symfony/polyfill-php54 dev-master b776342 Symfony polyfill backporting... symfony/polyfill-php55 dev-master 29b1381 Symfony polyfill backporting... symfony/polyfill-php56 dev-master e85ebde Symfony polyfill backporting... symfony/polyfill-php70 dev-master b6482e6 Symfony polyfill backporting... symfony/polyfill-util dev-master 67925d1 Symfony utilities for portab... symfony/psr-http-message-bridge dev-master b209840 PSR HTTP message bridge symfony/security-acl dev-master ab4dfe2 Symfony Security Component -... symfony/symfony v2.8.27 The Symfony PHP framework symfony/yaml 3.3.x-dev 8c7bf1e Symfony Yaml Component theseer/tokenizer 1.1.0 A small library for converti... twig/twig 2.x-dev d8d888e Twig, the flexible, fast, an... twilio/sdk 3.13.1 A PHP wrapper for Twilio's API webmozart/assert dev-master 53927dd Assertions to validate metho... webonyx/graphql-php dev-master eaadae4 A PHP port of GraphQL refere... willdurand/jsonp-callback-validator v1.1.0 JSONP callback validator. willdurand/negotiation 1.x-dev 2a59f23 Content Negotiation tools fo... zendframework/zend-diactoros 1.7.x-dev 89d471c PSR HTTP Message implementat...

I'm a new user, attempting an upgrade of phpunit from 6.3 to 7.3. I found that a few of our tests have started to fail. It seems they shouldn't, based on the statement that assertEquals uses == to check equality.

This fails:

$this->assertEquals('0', '0.00');

But:

php -a
Interactive mode enabled

php > var_dump('0' == '0.00');
php shell code:1:
bool(true)

Is this expected?

[IanVS]

I am also getting another (related?) problem when a + sign is in the string, which is exactly what the original issue I linked to said should not fail.

$this->assertEquals('+1', '1');

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'+1'
+'1'

[IanVS]

I think perhaps this is due to the change: #3179

[IanVS]

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 sebastianbergmann/comparator#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.

[sebastianbergmann]

ping @Ocramius

[IanVS]

@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');?

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?

[keradus]

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

[keradus]

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

[keradus]

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 ;)

[Ocramius]

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.

[Ocramius]

unexpected breaking change

This is a really important bugfix.
Yes, that means there's a possible BC break in bad tests, but I may even consider escalating it as security release, because having your tests fail and need adaptation (see #3185 (comment)) is still better than having a lurking bug not being exposed.

[Ocramius]

Note: self::assertEquals(new Comment('0.0'), new Comment('0.')) (yes, this is the golden example - see https://github.com/sebastianbergmann/comparator/pull/58/files for more examples) is not replaceable by self::assertSame(new Comment('0.0'), new Comment('0.')).

[keradus]

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 ?

[keradus]

@sebastianbergmann , how shall IsEqual constraint works?
Docs https://github.com/sebastianbergmann/phpunit/blob/master/src/Framework/Constraint/IsEqual.php#L19 is not matching implementation, and I believe this brought the confusion in this topic

[keradus]

ping @sebastianbergmann , can you take a look at this, please ?

[sebastianbergmann]

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

[stale]

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

[IanVS]

Hi, has any further thought been put into this? (I'm mostly jsut commenting to keep the issue open, because it seems there was no resolution.)

[Tobion]

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.

[Ocramius]

FWIW, either things are equalent or they aren't: you will need to make up your mind about whether your code follows this very fundamental principle or not, because `==` is most certainly no longer a suitable equivalence operator (or opcode) anymore anyway, as per findings in the test assets in here.

…

On Thu, 11 Oct 2018, 17:34 Tobias Schultze, ***@***.***> wrote: We are also affected by this heavy bc break. While I agree that the edge cases in #58 <#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. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3185 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAJakG73OfK6Ki-kh1rPiudasXyFVqc8ks5uj2TzgaJpZM4U3XOz> .

[Tobion]

In our case we used assertEquals for monetary values (represented as string obviously) and relied on the fact that trailing zeros are ignored (because the internal representation might use more decimals).

So the workaround given my @Ocramius to use $self::assertEquals(0, '0.00') does not work either because you don't represent money as float but as string. So both values are always string but now assertEquals complains about mismatching trailing zeros everywhere.

A solution would be to offer a new assertion that ignores trailing zeros on numeric values represented as string, which is the problem also reported by @IanVS

[stale]

This issue has been automatically marked as stale because it has not had activity within the last 60 days. It will be closed after 7 days if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed because it has not had activity since it was marked as stale. Thank you for your contributions.

[allan-simon]

sorry to dig up this issue, but it's the one google link to

shouldn't a specific assertBcEquals / assertDecimalEquals exists (I'm bad at naming) that under the hood use bccomp
so that it will works also for 7.10 and 7.1 ?
?