Use strpos instead of preg_match for responseContains/ResponseNotContains in WebAssert

[NickDickinsonWilde]

As far as I can tell, especially with the existence of WebAssert::responseMatches, it doesn't make sense to create a regex/use preg_match in responseContains/NotContains.

Maybe there is a reason for the use of preg_match there, but I did a search for prior issues/history and couldn't find any discussion. If there was and I missed it, a link would be great.,

[NickDickinsonWilde]

CI failing due to the tests. I had always assumed responseContains() was case sensitive. However, it clearly wasn't; so replacing strpos with stripos would allow the performance benefit without changing prior results.

[aik099]

The i modifier of preg_match was to match in case-insensitive manner, so to replicate that you need to use stripos instead of strpos.

[NickDickinsonWilde]

Yep, should've notice when I removed it.

[aik099]

The i modifier of preg_match was to match in case-insensitive manner, so to replicate that you need to use stripos instead of strpos.

[NickDickinsonWilde]

Remainder of issues Travis brings up appear to be some broken environments and unrelated to this pr.

[aik099]

Looks good to me.

Not sure if u modifier of preg_match (see http://php.net/manual/en/reference.pcre.pattern.modifiers.php) is even needed. As per docs it triggers a warning on malformed UTF-8 strings.

[stof]

thanks @NickWilde1990

[ghost]

Previously, if the $text is object, it was cast to the string:
$regex = '/'.preg_quote($text, '/').'/ui';

Now it casting to the int by doc:
$this->assert(strpos($actual, $text) !== FALSE, $message);

The result is an implicit regression with error like:

Object of class MySuperString could not be converted to int

Yes, the use of non-string types is contrary to the annotation of these methods. But it's very convenient when they have a suitable __string.

[aik099]

@vaplas , if the behavior isn't documented and it's just happens to work in unexpected way, to technically we're not breaking anything with this change.

Also you have 50%/50% chance of same Fatal Error, because not every object has __toString method.

[aik099]

Now it casting to the int by doc:

Not true. 1st and 2nd parameter of stripos or strpos are strings as well.

[ghost]

@aik099, thanks for the very quick feedback!

responseContains()/ResponseNotContains() have string $text type in annotations. So every objects must have the ability to string casting.

From strpos/stripos docs, 2nd parameter:

needle
If needle is not a string, it is converted to an integer and applied as the ordinal value of a character.

I fully agree with you that undocumented behavior should not be supported by strict backward compatibility policies.

I just wanted to explicitly point out this change, because in many cases, when working with methods responseContains()/ResponseNotContains(), string type on steroids can be used (for more convenient formatting, translations, etc.)

Although, of course, everyone can just wrap a WebAssert class. Or manually casting all the cases. But pure use (albeit undocumented) suffers.

[aik099]

needle
If needle is not a string, it is converted to an integer and applied as the ordinal value of a character.

Ha-ha. That must be an easter egg in PHP.

I just wanted to explicitly point out this change, because in many cases, when working with methods responseContains()/ResponseNotContains(), string type on steroids can be used (for more convenient formatting, translations, etc.)

Then your comment would guide other library users in right direction in case if they'll have that fatal error. Thank you.