Problem/Motivation

During Drupal 8, we added support for \Drupal::MINIMUM_SUPPORTED_PHP, which defines a PHP version that allows sites to run, but displays warnings on update and errors on the status report for existing sites. (New installations are not allowed except within the test runner.)

However, numerous tests now fail when MINIMUM_SUPPORTED_PHP is actually used, either due to issues with the test or possible regressions in HEAD.

One of these tests is Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest

Proposed resolution

Fix the tests.

This issue has three MRs:

  1. 3265376-FAIL lowers the minimum supported PHP (and skips a couple out-of-scope tests) to demonstrate the problem in HEAD.
  2. 3265376-update-script-test combines the above with a fix. Its PHP 7.3 test run should prove that the test now works on a version below MINIMUM_SUPPORTED_PHP.
  3. 3265376-fix-only is the version for commit, demonstrating that the fix alone also passes on HEAD.

Remaining tasks

Followups for other issues and to actually change the PHP version.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

Issue fork drupal-3265376

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm opened merge request !1837

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Active » Needs review

Unlike the other two, this one has a simple obvious fix.

xjm opened merge request !1838

xjm opened merge request !1839

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue summary: View changes

Explained the three MRs in the IS.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Related issues: +#3265377: Fix LocaleTranslatedSchemaDefinitionTest when MINIMUM_SUPPORTED_PHP is used, +#3265378: Fix NoPreExistingSchemaUpdateTest when MINIMUM_SUPPORTED_PHP is used
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs review » Needs work

Looks like there are a couple other tests to investigate.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs work » Needs review
Related issues: +#3265412: QuickStartTestBase is incompatible with the current implementation of MINIMUM_SUPPORTED_PHP

The other out-of-scope test failures are due to #3265412: QuickStartTestBase is incompatible with the current implementation of MINIMUM_SUPPORTED_PHP.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented

There we go. The FAIL MR demonstrates the bug with:

Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest
fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-1390.xml:
PHPUnit Test failed to complete; Error: PHPUnit 8.5.21 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest
................F                                                 17 / 17 (100%)

Time: 5.39 minutes, Memory: 6.00 MB

There was 1 failure:

1) Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest::testMaintenanceModeLink
The link maintenance mode was not found on the page.
Failed asserting that an array has the key 0.

/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:119
/var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/var/www/html/core/tests/Drupal/Tests/UiHelperTrait.php:503
/var/www/html/core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php:664
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:703

FAILURES!
Tests: 17, Assertions: 473, Failures: 1.

The combined MR passes on PHP 7.3 and proves the fix addresses the failure of this particular test, and the fix-only patch is committable as it passes on both PHP 8.0 and PHP 7.3.

This is ready for review.

daffie’s picture

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

The code changes look good to me.
The combination of the Mr that fails and the MR with the fix convincing.
For me it is RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 3f55b8268f2 to 10.0.x and ffdbfa52fe1 to 9.4.x and a17f93cce31 to 9.3.x. Thanks!

  • alexpott committed 3f55b82 on 10.0.x 
    Issue #3265376 by xjm: Fix UpdateScriptTest when MINIMUM_SUPPORTED_PHP...

  • alexpott committed ffdbfa5 on 9.4.x 
    Issue #3265376 by xjm: Fix UpdateScriptTest when MINIMUM_SUPPORTED_PHP...

  • alexpott committed a17f93c on 9.3.x 
    Issue #3265376 by xjm: Fix UpdateScriptTest when MINIMUM_SUPPORTED_PHP...

xjm closed merge request !1837

xjm closed merge request !1838

xjm closed merge request !1839

Status: Fixed » Closed (fixed)

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