Tests failures on PHP 7.2.x

I probably should go one by one with each issue so jrockowitz has better time following the findings and guiding me should I take any wrong decision.

So, let me start with Drupal\webform\Tests\Element\WebformElementComputedTest. The testbot complains:

exception: [Warning] Line 51 of sites/simpletest/63888434/files/php/twig/5a6e683c4273d_inline-template_YdT8JwRx-PzbiUiRVsLMA4aL6/h28BDtk14cdKPdo4hvVKSDuxwUze5DqCb4iHsGSnvM8.php:
A non-numeric value encountered

This issue is reproducible since PHP 7.1. The changelog: http://php.net/manual/en/migration71.other-changes.php which states that PHP will complain if we ask it to execute a math operation where one operand is a string that does not contain a well formatted number.

The webform webform.webform.test_element_computed_twig.yml contains an inline Twig template with the following snippet: {{ webform_token('[webform_submission:values:number]', webform_submission) * 2 }}. It will execute just fine when you preview or view a webform submission. But if you just open the webform (and you're about to submit something into it), the token cannot determine the value as the webform submission data is still empty. Thus PHP effectively is asked to execute (string) '[webform_submission:values:number]' * 2 and rightfully it complains.
The rendered template is hidden on the form view via #access => FALSE (it becomes #access => TRUE when we view a submission). But just placing access FALSE is not enough because the rendering still happens. The solution is not to invoke Twig engine at all if #access is FALSE.. anyway Twig's work will not be shown to the end user.

I attach the patch that does it.

Implications of the bug: As much as I can judge, from end user perspective there was no real deficiency in performance/features. It's just an internal cleanup.

PHP 7.1.* will complain if you do

echo "" * 5;

but the issue is addressed in my patch #2.

Now I am addressing an issue in WebformEntityElementsValidationTest. I didn't manage to reproduce it locally but from analyzing the code and the error message, seems like it was just a misspelling in the test case source code. I am uploading the patch to let drupal testbot confirm my assumption about the misspelling.

Alright :) Yes, I took your patch and incorporated into my other progress + reverted my fix through #access flag. Attaching the patch.

Okie dokie, the next guy is ./webform/modules/webform_ui/src/Tests/WebformUiElementTest.php This one fails not because of PHP 7 but because of Drupal core 8.5. WIth D8.5 they have stabilized off_canvas dialogs so webform UI changes a little bit the data-dialog-* attributes in the links it generates. The failing test had a too restrictive match and because of the change began to fail on D8.5. I only had to relax a little bit your assert condition. Probably a quick glance at the interdiff.txt will tell you more than my English.

The last failing test turned out to be a relatively serious thing. Hehe, let me jump the gun and say that because of those PHP warnings/notices introduced in PHP 7.1 the things I am about to say became visible.

The function _webform_entity_element_validate_rendering_error_handler() contained a logical error. Probably a typo. Let me just paste here in-line the interdiff.txt:

@@ -740,7 +740,7 @@ function _webform_entity_element_validate_rendering_error_handler($error_level,
     throw new ErrorException($message, $error_level, 0, $filename, $line);
   }
   else {
-    _drupal_error_handler($message, $message, $filename, $line, $context);
+    _drupal_error_handler($error_level, $message, $filename, $line, $context);
   }
 }

Instead of supplying the $error_level the $message was supplied. This led drupal standard error handler to believe that the error was not important and it was not logging it anywhere. Some [seems to me] significant errors were unintentionally suppressed that way. One of those is: call_user_func_array() expects parameter 1 to be a valid callback, class 'Drupal\webform\Element\WebformImageFile' does not have a method 'processManagedFile'. Maybe there are more. I am uploading a patch and will see if drupal test bot finds fails on more of them.

After fixing the issue of suppressing errors, an additional error got revealed in the same WebformUiElementPropertiesTest test.

I am attaching a patch that fixes it. The error was because the test did not install a 'file' core module while poking file-based element types. I also include a few lines comment about a not-so-trivial IF statement because it took me a few hours to "get" it :)

I hope this time we are going to see a 100% pass :)