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 for #4368: Add tests for LogicalNot::negate, fixes to negate regex #4479

Closed
wants to merge 2 commits into from

Conversation

kamioftea
Copy link

This is a partial fix for the issue raised in #4368 that

  • Adds the tests suggested by @markrogoyski
  • Fixes the existing regex to match single quotes for both strings
  • Allows the match of the first single quote enclosed strings to be optional, so this also works for the __toString() use case

The above fixes to the regex allow the suggested tests to pass, however test strings with single quotes in them still show incorrect behaviour, i.e. the following test still fails:

public function testConstraintIsNotEqualStringContainsSingleQuotes(): 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();
}

@kamioftea kamioftea changed the title Add tests for LogicalNot::negate, fixes to negate regex Fix for #4368: Add tests for LogicalNot::negate, fixes to negate regex Oct 3, 2020
@codecov
Copy link

codecov bot commented Oct 3, 2020

Codecov Report

Merging #4479 (93b3986) into 8.5 (1615054) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                8.5    #4479   +/-   ##
=========================================
  Coverage     84.17%   84.17%           
  Complexity     3970     3970           
=========================================
  Files           154      154           
  Lines         10172    10172           
=========================================
  Hits           8562     8562           
  Misses         1610     1610           
Impacted Files Coverage Δ Complexity Δ
src/Framework/Constraint/LogicalNot.php 100.00% <100.00%> (ø) 16.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1615054...93b3986. Read the comment docs.

@sebastianbergmann sebastianbergmann added feature/assertion Issues related to assertions and expectations type/bug Something is broken labels Oct 4, 2020
@sebastianbergmann
Copy link
Owner

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it.

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

Successfully merging this pull request may close these issues.

2 participants