mb_strpos(): Empty delimiter

[GaryJones]

This assertion:

self::assertStringContainsString( $options['apikey'], $registry['parsely-analytics-for-wordpress']['payload'] );

found in the test files of this PR (work in progress, so may change), gave a failure in a GitHub Action when run under PHP 5.6.40 and PHPUnit 5.7.27, with an error message of mb_strpos(): Empty delimiter.

It points to this part of this repo, which then appears to rely on PHPUnit itself (assertContains()).

The next PHP up in my workflow is PHP 7.0.33, and that uses PHPUnit 6.5.14, and that works fine.

This issue in PHPUnit seems highly relevant, so I think I'm looking for a back-compat fix for assertContains() for PHPUnit 5.7.

Could this be added in please @jrfnl?

[GaryJones]

The $options['apikey'] value was an empty string, which was an oversight on my part, but still highlights a discrepancy between different PHPUnit versions.

[jrfnl]

@GaryJones I don't think this repo can - nor should - work around any and all bugs in PHPUnit native assertions found and fixed between PHPUnit 4.x and now.

Having said that, this is such a simple fix, that I'd be happy to accept a patch for it (with a unit test).

As you've already done all the necessary research to find the underlying cause and how this was patched upstream, would you be willing to submit a PR ?

The $options['apikey'] value was an empty string, which was an oversight on my part, but still highlights a discrepancy between different PHPUnit versions.

And it looks like we were looking at the same thing just now, as I was just about to ask you how that test could ever hit that condition as the $needle parameter in neither assertions seemed to be an empty string. And if it were, a different assertion might be more appropriate.

[GaryJones]

would you be willing to submit a PR?

Would it just be a case of adding an assertContains() method, with the extra fix / pull the method from PHPUnit 9 and remove any syntax incompatible down to PHP 5.6?

[jrfnl]

Would it just be a case of adding an assertContains() method, with the extra fix / pull the method from PHPUnit 9 and remove any syntax incompatible down to PHP 5.6?

I'd suggest to keep the patch much simpler:

• Add a check against an empty string at the top of the relevant assertStringContainsString() variant functions in the AssertStringContains trait and handle that situation the same as PHPUnit does - similar to the commit you referenced in the original issue description.
• Add a test for each of the updated functions to the test file for the trait.

Reasoning: this library does not polyfill assertContains() as that function is available cross-version in PHPUnit, albeit that the behaviour of that function changed across versions.
What we are polyfilling is the new assertStringContainsString() functionality which is the replacement for functionality removed from assertContains().

If people use this library the way it is intended to be used, i.e. use PHPUnit 9.x syntax and run the tests in all supported PHPUnit versions, only the assertStringContainsString() functionality will be affected by this bug, as when an array haystack is passed to assertContains(), the mb_strpos() won't be hit and when a string haystack is used, people would have gotten the PHPUnit deprecation notice and should have switched to the assertStringContainsString() function as polyfilled by this library.

Does that make sense ?