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

Assertion assertNotEquals Provides Wrong Error Message On Failure #4368

Closed
markrogoyski opened this issue Jul 8, 2020 · 1 comment
Closed
Labels
feature/assertion Issues related to assertions and expectations type/bug Something is broken

Comments

@markrogoyski
Copy link
Contributor

Q A
PHPUnit version 8.5.8, 9.3-g33d19b2aa,
PHP version 7.3.3, 7.4.7
Installation Method Composer

Summary

The assertNotEquals test assertion will produce an incorrect error message on failure when asserting a string is not equal to another string in the following cases:

  • String has double quotes in it
  • String has "positive" words in it

In this case, failure of assertNotEquals means that the strings are equal.
"Positive" words are those that PHPUnit\Framework\Constraint\LogicalNot::negate will translate to "negative" words.

Current behavior

In the case that the string has double quotes in it, the error message will say: Failed asserting that '$STRING' is equal to '$STRING'., when the expected error message would be Failed asserting that '$STRING' is not equal to '$STRING'.

In the case that the string has "positive" words in it, you get nonsense error messages where the string is "negated."

How to reproduce

Here is a basic test case that works as expected.

class NotEqualsTest extends \PHPUnit\Framework\TestCase
{
    public function testNotEqualString(): void
    {
        // Given
        $string = 'a b c';

        // Then
        $this->assertNotEquals($string, $string);
    }
}

And it produces the expected error message "is not equal":

Failed asserting that 'a b c' is not equal to 'a b c'.

However, now we do the same simple test case but with double quotes appearing in it:

class NotEqualsTest extends \PHPUnit\Framework\TestCase
{

    public function testNotEqualStringWithDoubleQuotes(): void
    {
        // Given
        $string = 'a "b" c';

        // Then
        $this->assertNotEquals($string, $string);
    }
}

And it produces an unexpected error message that it failed to assert that it "is equal":

Failed asserting that 'a "b" c' is equal to 'a "b" c'.

Here is another example with a JSON string.

class NotEqualsTest extends \PHPUnit\Framework\TestCase
{
    public function testNotEqualJsonString(): void
    {
        // Given
        $string = '{"key":"value"}';

        // Then
        $this->assertNotEquals($string, $string);
    }
}

And the unexpected error message:

Failed asserting that '{"key":"value"}' is equal to '{"key":"value"}'.

Things get really strange when the string has "positive" words in it. They all become "negated" in the error message. Here is an example test case:

class NotEqualsTest extends \PHPUnit\Framework\TestCase
{
    public function testNotEqualNegatingTheValueNoQuotes(): void
    {
        // Given
        $string = 'This starts with a reference that is affirmative and ends with a reference that contains and has another affirmative';

        // Then
        $this->assertNotEquals($string, $string);
    }
}

And the wildly unexpected error message:

Failed asserting that 'This starts not with a don't reference that is not affirmative and ends not with a don't reference that does not contain and does not have another affirmative' is not equal to 'This starts not with a don't reference that is not affirmative and ends not with a don't reference that does not contain and does not have another affirmative'.

Expected behavior

The expected behavior is that the error message would say "is not equal" and "positive" words would not be "negated" in the values being asserted on.

Root Cause

Both errors seem to be originating in the negate function in PHPUnit\Framework\Constraint\LogicalNot. On line 48 there is this regular expression:

preg_match('/(\'[\w\W]*\')([\w\W]*)("[\w\W]*")/i', $string, $matches);

The intent of this regular expression is not immediately clear and it is not documented, but this is looking for something specific and is catching things it should not catch, such as the strings with quotes in them.

Immediately after that is the preg_replace of "positives" to "negatives." It seems this is presuming that those positive words will not appear in the value being asserted against. Just make a test case containing only the positive words and you will see every single one be negated in the error message:

class NotEqualsTest extends \PHPUnit\Framework\TestCase
{
    public function testNotEqualStringWithAllPositivesTurningToNegatives(): void
    {
        // Given
        $string = 'contains exists has is are matches starts with ends with reference not not positive';

        // Then
        $this->assertNotEquals($string, $string);
    }
}

Resulting error message:

Failed asserting that 'does not contain does not exist does not have is not are not does not match starts not with ends not with don't reference not positive' is not equal to 'does not contain does not exist does not have is not are not does not match starts not with ends not with don't reference not positive'.

Fix

It's not immediately clear what a quick fix for this problem is without doing some redesign of how these error messages get generated. Trying to parse the string knowing it may contain quotes and the "positive" words seems error prone. I apologize that I am not providing a pull request to fix this.

Here are some PHPUnit framework unit tests you can use as a starting point to TDD the problem, starting from a failing test that exhibits the issues.

In unit test PHPUnit\Framework\ConstraintTest, you can add the following test case, following the pattern of tests in the same area of the code:

    public function testConstraintIsNotEqualStringContainsQuotes(): void
    {
        $string = 'a "b" c';
        $other = 'a ""b"" c';
        $constraint = Assert::logicalNot(
            Assert::equalTo($string)
        );

        $this->assertTrue($constraint->evaluate($other, '', true));
        $this->assertFalse($constraint->evaluate($string, '', true));
        $this->assertEquals("is not equal to '$string'", $constraint->toString());
        $this->assertCount(1, $constraint);

        try {
            $constraint->evaluate($string);
        } catch (ExpectationFailedException $e) {
            $this->assertEquals(
                <<<EOF
Failed asserting that '$string' is not equal to '$string'.

EOF
                ,
                TestFailure::exceptionToString($e)    // Fails here
            );

            return;
        }

        $this->fail();
    }

It will fail with the following error message:

1) PHPUnit\Framework\ConstraintTest::testConstraintIsNotEqualStringContainsQuotes
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'Failed asserting that 'a "b" c' is not equal to 'a "b" c'.\n
+'Failed asserting that 'a "b" c' is equal to 'a "b" c'.\n
 '

phpunit/src/Framework/Constraint/Equality/IsEqual.php:102
phpunit/src/Framework/Assert.php:2290
phpunit/src/Framework/Assert.php:322
phpunit/tests/unit/Framework/ConstraintTest.php:408
phpunit/src/Framework/TestCase.php:1504
phpunit/src/Framework/TestCase.php:1110
phpunit/src/Framework/TestResult.php:733
phpunit/src/Framework/TestCase.php:851
phpunit/src/Framework/TestSuite.php:665
phpunit/src/TextUI/TestRunner.php:689
phpunit/src/TextUI/Command.php:134
phpunit/src/TextUI/Command.php:94

And a similar test case for positive words in the string being asserted on:

    public function testConstraintIsNotEqualStringContainsPositiveWords(): void
    {
        $string = 'a is b';
        $other = 'a certainly is b';
        $constraint = Assert::logicalNot(
            Assert::equalTo($string)
        );

        $this->assertTrue($constraint->evaluate($other, '', true));
        $this->assertFalse($constraint->evaluate($string, '', true));
        $this->assertEquals("is not equal to '$string'", $constraint->toString());  // Fails here
        $this->assertCount(1, $constraint);

        try {
            $constraint->evaluate($string);
        } catch (ExpectationFailedException $e) {
            $this->assertEquals(
                <<<EOF
Failed asserting that '$string' is not equal to '$string'.

EOF
                ,
                TestFailure::exceptionToString($e)
            );

            return;
        }

        $this->fail();
    }

It will fail with the following error message:

2) PHPUnit\Framework\ConstraintTest::testConstraintIsNotEqualStringContainsPositiveWords
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'is not equal to 'a is b''
+'is not equal to 'a is not b''

phpunit/src/Framework/Constraint/Equality/IsEqual.php:102
phpunit/src/Framework/Assert.php:2290
phpunit/src/Framework/Assert.php:322
phpunit/tests/unit/Framework/ConstraintTest.php:427
phpunit/src/Framework/TestCase.php:1504
phpunit/src/Framework/TestCase.php:1110
phpunit/src/Framework/TestResult.php:733
phpunit/src/Framework/TestCase.php:851
phpunit/src/Framework/TestSuite.php:665
phpunit/src/TextUI/TestRunner.php:689
phpunit/src/TextUI/Command.php:134
phpunit/src/TextUI/Command.php:94
@markrogoyski markrogoyski added the type/bug Something is broken label Jul 8, 2020
@sebastianbergmann sebastianbergmann added the feature/assertion Issues related to assertions and expectations label Jul 8, 2020
@sebastianbergmann
Copy link
Owner

Superseded by #5516.

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

2 participants