[PHP 7.3] strpos explicit string needle warnings

I don't think the latest patch is the intended behavior for assertRaw. The method is for checking if a value is displayed at the page result. New behavior checks if the chr result is displayed, which doesn't equal the integer. E.g. chr(98) equals 'b'

Also this fix doesn't check for other types like floats. For instance in the following test is assert that floating number -1234.56 is displayed at the page: https://cgit.drupalcode.org/drupal/tree/modules/field/modules/number/num...

To avoid this and keep the current behavior we should cast the second argument passed to strpos ($raw) to a string.

Hi @Ayesh, my guess is the patch in #2 is correct and the failing test may be broken all along.
Just tried the strpos() method for a float needle and it seems like the decimals aren't accounted for! Check this test code snippet: https://3v4l.org/sT9CU
Seems like the test without string cast accepts all texts containing a dot.

Hi all,

I'm testing the patches locally and I re-rolled a version of the patch.

I will consolidate all the PHP 7.3 patches in a custom project of mine that you can find here: https://github.com/drupol/dockerdrupal7runnerngninxphpfpm

Basically, it allows you to test D7 with docker without changing anything locally.
With only 3 commands, you can get a D7 site setup with PHP 7.3.

Feel free to use it!

Looks good to me! +1
The check in method assertRaw is now correct for decimal numbers and the bug in the number test has been fixed.

Did a quick check in D8: This bug has already been fixed there: #2279339: NumberFieldTest is slightly and subtly broken

👍 Looks good to me, I reran the 7.3 tests and now down to just #3009351: "session_id(): Cannot change session id"

Cool ! Moving this forward.

+++ b/modules/field/modules/number/number.test
@@ -69,7 +69,7 @@ class NumberFieldTestCase extends DrupalWebTestCase {
-    $this->assertRaw(round($value, 2), 'Value is displayed.');
+    $this->assertRaw(round($value, 4), 'Value is displayed.');

We should be fixing this in the same way as Drupal 8 did, e.g. just removing the round completely to match code and make further syncs easier.

The rest is good to go though.

Patch updated based on feedback from #13.

Looking good!

Updating credits.

This is fixed, thanks again everyone!

Removing the pending commit tag (it was already committed).