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

Handle strings containing empty strings assertions #18

Merged
merged 3 commits into from
Jun 8, 2021

Commits on Jun 5, 2021

  1. Handle strings containing empty strings assertions

    PHPUnit 6.4.2 added a fix to gracefully handle the case when an empty string was passed to `assertStringContainsString()` and variant assertions. Versions earlier than that gave a "mb_strpos(): Empty delimiter" error instead.
    
    This change adds a fix to also handle that case, when using PHPUnit-Polyfills with versions of PHPUnit earlier than 6.4.2, and passing an empty needle string.
    
    The use of asserting true is true stops the test from being marked as risky, which is what would happen if we just `return true;`.
    GaryJones committed Jun 5, 2021
    Configuration menu
    Copy the full SHA
    aa1db10 View commit details
    Browse the repository at this point in the history
  2. Handle empty string assertions by skipping tests

    - The backfill trait only applies for PHPUnit < 7.5.0
    - The empty string handling was added in PHPUnit 6.4.2.
    - Assertions with empty strings run with PHPUnit below 6.4.2 always need the fix, giving a skipped test.
    - Assertions with empty strings run with PHPUnit 6.4.2 or above, don't need the fix, unless using a negative assertion, since an empty string will always exist in another string, hence the NotContains will always fail.
    GaryJones committed Jun 5, 2021
    Configuration menu
    Copy the full SHA
    7b32021 View commit details
    Browse the repository at this point in the history

Commits on Jun 8, 2021

  1. AssertStringContains: finish off the fix for mb_strpos() warning

    As per Yoast#18 (comment). it is okay for the polyfills to work around this particular bug, but the work-around should not change the behaviour of the assertions.
    
    With that in mind, I've made the following changes:
    
    ### `AssertStringContains` trait
    
    * Removed the test skipping, in favour of emulating the PHPUnit >= 6.4.2 behaviour.
        A test may be doing multiple assertions. If this particular assertion would cause the test to be skipped, the other assertions (if after this assertion) would not be run anymore. This would be a change in behaviour, which is undesirable.
    * As the behaviour of the underlying assertion has not changed significantly between PHPUnit 6.4.2 < 7.5.0, the version check is not really needed and removing it simplifies the fix and makes it more performant while not impacting the functionality.
    * Moved the documentation for the fix to the class docblock as the `needsEmptyStringFix()` method has been removed.
    
    Notes:
    * There is no `return` after the calls to `fail()` as when the assertion fails the test, it throws an exception which will automatically stop the execution of the function.
    * Also note the use of `static::` rather than `self::` to call PHPUnit native functionality. This allows for existing method overloads in a child class of the PHPUnit native `TestCase` to be respected.
    
    ### Tests
    
    * Expanded the documentation for the tests so it will always be clear that these tests are for a specific fix in the polyfills.
    * Added a dataprovider for two of the tests to allow for testing with different haystacks, making sure a certain edge case doesn't give any issues.
    * Updated the tests for the `assertStringNotContainsString()` polyfills to expect a `PHPUnit\Framework\AssertionFailedError` exception.
    jrfnl committed Jun 8, 2021
    Configuration menu
    Copy the full SHA
    0611ef2 View commit details
    Browse the repository at this point in the history