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

StringMatchesFormatDescription Does Not Handle Escaped % Properly #2907

Closed
mkasberg opened this issue Dec 7, 2017 · 2 comments
Closed

StringMatchesFormatDescription Does Not Handle Escaped % Properly #2907

mkasberg opened this issue Dec 7, 2017 · 2 comments

Comments

@mkasberg
Copy link
Contributor

mkasberg commented Dec 7, 2017

Q A
PHPUnit version 6.5.3
PHP version 7.0.22-0ubuntu0.17.04.1
Installation Method Git

I would expect the following test case to pass because the format string '%%' should produce a literal '%' (sprintf). But it fails in PHPUnit 6.5.3.

StringMatchesFormatDescriptionTest.php

<?php

use PHPUnit\Framework\TestCase;

class StringMatchesFormatDescriptionTest extends TestCase {

    public function testEscapedPercent() {
        $this->assertStringMatchesFormat('This %%d is escaped.', 'This %d is escaped.');
    }

}

Sample Output:

$ ./phpunit StringMatchesFormatDescriptionTest.php 
PHPUnit 6.5.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.0.22-0ubuntu0.17.04.1 with Xdebug 2.5.0
Configuration: /home/mkasberg/repositories/phpunit/phpunit.xml

F                                                                   1 / 1 (100%)

Time: 436 ms, Memory: 4.00MB

There was 1 failure:

1) StringMatchesFormatDescriptionTest::testEscapedPercent
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
-This %%d is escaped.
+This %d is escaped.

/home/mkasberg/repositories/phpunit/src/Framework/Constraint/Constraint.php:117
/home/mkasberg/repositories/phpunit/src/Framework/Constraint/Constraint.php:62
/home/mkasberg/repositories/phpunit/src/Framework/Assert.php:2116
/home/mkasberg/repositories/phpunit/src/Framework/Assert.php:1768
/home/mkasberg/repositories/phpunit/StringMatchesFormatDescriptionTest.php:8
/home/mkasberg/repositories/phpunit/src/Framework/TestCase.php:1071
/home/mkasberg/repositories/phpunit/src/Framework/TestCase.php:939
/home/mkasberg/repositories/phpunit/src/Framework/TestResult.php:698
/home/mkasberg/repositories/phpunit/src/Framework/TestCase.php:894
/home/mkasberg/repositories/phpunit/src/Framework/TestSuite.php:755
/home/mkasberg/repositories/phpunit/src/TextUI/TestRunner.php:546
/home/mkasberg/repositories/phpunit/src/TextUI/Command.php:195
/home/mkasberg/repositories/phpunit/src/TextUI/Command.php:148

FAILURES!
Tests: 1, Assertions: 1, Failures: 1.

I skimmed the source code for StringMatchesFormatDescription.php, and it is perhaps worth noting that we also seem to be missing a few other descriptors which are supported by PHP. For example, %b, %E, and %g. (Should we expect PHPUnit to support all of these?)

None of these are major bugs since it's certainly possible to work around them, but it might be nice to have better support for format string matching.

@sebastianbergmann
Copy link
Owner

sebastianbergmann commented Dec 7, 2017

I would consider (separate) pull requests that fix % escaping and add missing formatting placeholders.

@mkasberg
Copy link
Contributor Author

mkasberg commented Dec 7, 2017

I think I could take a stab at this. I'll see what I can come up with...

mkasberg added a commit to mkasberg/phpunit that referenced this issue Dec 9, 2017
For example, the format '%%d' should match the string literal '%d'.

We achieve this by using negative lookbehind
(http://php.net/manual/en/regexp.reference.assertions.php) when
constructing the regex pattern that we will match against.
mkasberg added a commit to mkasberg/phpunit that referenced this issue Dec 9, 2017
For example, the format '%%d' should match the string literal '%d'.

We achieve this by using negative lookbehind
(http://php.net/manual/en/regexp.reference.assertions.php) when
constructing the regex pattern that we will match against.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants