Problem/Motivation

D7 tests with PHP 8.0 currently hit these failures:

Drupal error handlers 22 passes, 8 fails, and 0 exceptions

The test class is: Drupal error handlers (DrupalErrorHandlerTestCase)

Steps to reproduce

Looking at e.g. https://www.drupal.org/pift-ci-job/2017309 the failures are:

testErrorHandler

fail: [Browser] Line 45 of modules/simpletest/tests/error.test:
Received expected HTTP status code.

fail: [Other] Line 46 of modules/simpletest/tests/error.test:
Found error message: Notice: Undefined variable: bananas in error_test_generate_warnings() (line .

fail: [Other] Line 47 of modules/simpletest/tests/error.test:
Found error message: Warning: Division by zero in error_test_generate_warnings() (line .

fail: [Other] Line 48 of modules/simpletest/tests/error.test:
Found error message: User warning: Drupal is awesome in error_test_generate_warnings() (line .

fail: [Browser] Line 53 of modules/simpletest/tests/error.test:
Received expected HTTP status code.

fail: [Other] Line 55 of modules/simpletest/tests/error.test:
Found error message: Warning: Division by zero in error_test_generate_warnings() (line .

fail: [Other] Line 56 of modules/simpletest/tests/error.test:
Found error message: User warning: Drupal is awesome in error_test_generate_warnings() (line .

fail: [Browser] Line 61 of modules/simpletest/tests/error.test:
Received expected HTTP status code.

Proposed resolution

Fix failing tests.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3206429

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mcdruid created an issue. See original summary.

mcdruid’s picture

mcdruid commented
Issue summary: View changes
mcdruid’s picture

mcdruid commented
Issue summary: View changes
mcdruid’s picture

mcdruid commented
Parent issue: » #3145797: [META] Make Drupal 7 core compatible with PHP 8.0

Taran2L made their first commit to this issue’s fork.

Taran2L opened merge request !476

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Active » Needs review
Related issues: +#3206430: [PHP 8] test failures in SimpleTest functionality

This issue as well as #3206430: [PHP 8] test failures in SimpleTest functionality are both rely on the same piece of code in error_test.module. Thus, I suggest fixing both here.

The main issue here is the tests doing checks that are not true anymore in PHP8+, mostly due to the fact that a lot of things are being converted to exceptions, some are "promoted" to a different severity.

One by one:

  • Undefined variable notices became a warning in PHP8+
  • Division by zero warning became a DivisionByZeroError exception
  • Passing wrong or missing parameters throws TypeError or ArgumentCountError exceptions

Basically, I've copied code that produces PHP notices/warnings across PHP5.3 - 8.0 from D8 (looking for the issue ...)

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs review » Needs work 
  $obj = new \stdClass();
  $obj->p =& $obj;
  var_export($obj, TRUE);

this piece does not work in PHP 5.3 and PHP5.4.0-5.4.6, probably a better warning generating issue is needed

Also, I'm not sure about 

    // Generates 3 warnings inside a PHP function.
    simplexml_load_string('<fake>');

as it does produce 3 PHP warnings, not 1 ...

Does anyone have a better example?

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented

So, thanks to @chmez, figured out a piece of code that produces a PHP warning across all the supported versions (except the message is not the same, but I think we can leave with it):

  $a = '';
  foreach ($a as $b) {

  }

.. a PHP function that generates a single warning is still to do

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs work » Needs review

But in the end, it does matter that all warnings are being caught not their number

mcdruid’s picture

mcdruid commented

This looks pretty good.

I am wondering whether we'd be better to leave the existing code in place and have new versions of everything for PHP 8 and above... but if you've managed to find examples that produce the appropriate warnings/notices etc.. right back to PHP 5.3 perhaps we don't need to add messy logic to keep the old code.

Maybe I'd just be sad to say goodbye to the monkeys and their $bananas :)

I might have a quick look at how ugly it'd get to branch differently for PHP 8, but generally it looks like you've done a good job of updating everything.

  • mcdruid committed 4c11b77 on 7.x 
    Issue #3206429 by Taran2L, chmez: [PHP 8] test failures in Drupal error...

mcdruid credited chmez.

mcdruid’s picture

mcdruid commented
Status: Needs review » Reviewed & tested by the community

Discussed with @Fabianx who was happy with these changes.

Sorry monkeys :)

mcdruid credited Fabianx.

mcdruid’s picture

mcdruid commented
Status: Reviewed & tested by the community » Fixed

Excellent - so that's PHP 8 tests passing.

Thank you everyone!

mcdruid closed merge request !476

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.