Allow E_DEPRECATED deprecations to be skipped so we can test on PHP 8.1 [#3240456]

Problem/Motivation

In order to be able to use Guzzle 6 on PHP 8.1 we need the ability to skip E_DEPRECATED deprecations so we can skip:
Return type of GuzzleHttp\Cookie\CookieJar::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
Return type of GuzzleHttp\Cookie\CookieJar::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
Unfortunately it doesn't seem that the Guzzle team want to release a new version of Guzzle 6. We will upgrade to Guzzle 7 in Drupal 10 and might change our dependencies to allow Guzzle 7 with Drupal 9 but this is the easiest fix to do for PHP 8.1 compatibility in Drupal 9.3.

Steps to reproduce

Run (for example) Drupal\Tests\content_translation\Functional\ContentTranslationSettingsTest

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comment	File	Size	Author
#11	3240456-11.patch	4.01 KB	alexpott
#11	9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass
#11	3-11-interdiff.txt	1.26 KB	alexpott
#3	3240456-3.patch	4.31 KB	alexpott
#3	9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass PHP 8.1 & sqlite-3.27 Build Successful
#3	3240456-3.test-only.patch	1.15 KB	alexpott
#3	9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass PHP 8.1 & sqlite-3.27 Build Successful
#3	2-3-interdiff.txt	1.87 KB	alexpott
#2	3240456-2.patch	2.41 KB	alexpott
#2	9.3.x: PHP 8.0 & MySQL 5.7 28,910 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

3 October 2021 at 10:55

alexpott created an issue. See original summary.

Comment #2

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 3 October 2021 at 11:04

Status:

Active

» Needs review

File	Size
3240456-2.patch	2.41 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,910 pass

Patch attached is from the meta. It allows us to skip deprecations caused by Guzzle and EasyRDF. EasyEDF has a PR to fix PHP 8.1 deprecations but that's not landed yet. If it does we can remove the skip.

I'm thinking about the best way to add a test for this. The fact that tests fail on PHP 8.1 is a kind of test but it might be good to have something explicit. User-land code is not allowed to do trigger_error('some text', E_DEPRECATED) - you get a warning... Warning: Uncaught ValueError: trigger_error(): Argument #2 ($error_level) must be one of E_USER_ERROR, E_USER_WARNING, E_USER_NOTICE, or E_USER_DEPRECATED

Comment #3

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 3 October 2021 at 11:26

File	Size
2-3-interdiff.txt	1.87 KB
3240456-3.test-only.patch	1.15 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass PHP 8.1 & sqlite-3.27 Build Successful
3240456-3.patch	4.31 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass PHP 8.1 & sqlite-3.27 Build Successful

Here's a test. Seems okay to add this note it will only trigger the deprecation on PHP 8.1 so to prove it fails there you need to run the test manually at this point we still are triggering too many deprecation on PHP 8.1.

Here's the output of running the test without the changes to \Drupal\Tests\Listeners\DeprecationListenerTrait...

./vendor/bin/phpunit -v core/tests/Drupal/Tests/SkippedDeprecationTest.php 
PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.0RC3-dev
Configuration: /Users/alex/dev/sites/drupal8alt.dev/phpunit.xml
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Testing Drupal\Tests\SkippedDeprecationTest
...                                                                 3 / 3 (100%)

Time: 00:00.025, Memory: 10.00 MB

OK (3 tests, 3 assertions)

Remaining self deprecation notices (1)

  1x: Return type of PhpDeprecation::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in SkippedDeprecationTest::testSkippingPhpDeprecations from Drupal\Tests

Comment #4

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 3 October 2021 at 13:14

Arggh the mink deprecations get in the way...

Comment #5

andypost

he/him

Russian

CreditAttribution: andypost as a volunteer and at Skilld commented 3 October 2021 at 13:21

Status:

Needs review

» Reviewed & tested by the community

1 file was hidden/shown/deleted

File	Size
3240456-2.patch	2.41 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,910 pass

Thank you, this patch makes it easy to run all other tests

Running test with test-only patch

vendor/bin/phpunit -c core/phpunit.xml.dist --colors=always --debug -v \
core/tests/Drupal/Tests/SkippedDeprecationTest.php
PHPUnit 8.5.21 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.0RC3
Configuration: /var/www/html/web/core/phpunit.xml.dist

Testing Drupal\Tests\SkippedDeprecationTest
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecations' started
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecations' ended
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecationsAgain' started
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecationsAgain' ended
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingPhpDeprecations' started
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingPhpDeprecations' ended


Time: 566 ms, Memory: 6.00 MB

OK (3 tests, 3 assertions)

Remaining self deprecation notices (1)

  1x: Return type of PhpDeprecation::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
    1x in SkippedDeprecationTest::testSkippingPhpDeprecations from Drupal\Tests

using patch it pass

vendor/bin/phpunit -c core/phpunit.xml.dist --colors=always --debug -v \
core/tests/Drupal/Tests/SkippedDeprecationTest.php
PHPUnit 8.5.21 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.1.0RC3
Configuration: /var/www/html/web/core/phpunit.xml.dist

Testing Drupal\Tests\SkippedDeprecationTest
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecations' started
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecations' ended
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecationsAgain' started
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingDeprecationsAgain' ended
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingPhpDeprecations' started
Test 'Drupal\Tests\SkippedDeprecationTest::testSkippingPhpDeprecations' ended


Time: 292 ms, Memory: 6.00 MB

OK (3 tests, 3 assertions)

Comment #6

andypost

he/him

Russian

CreditAttribution: andypost as a volunteer and at Skilld commented 3 October 2021 at 13:24

Status:

Reviewed & tested by the community

» Needs review

ignore this comment

Comment #7

andypost

he/him

Russian

CreditAttribution: andypost as a volunteer and at Skilld commented 3 October 2021 at 13:30

Running core/modules/content_translation/tests/src/Functional/ContentTranslationSettingsTest.php

OK (4 tests, 103 assertions)

Unsilenced deprecation notices (184)

  181x: strtolower(): Passing null to parameter #1 ($string) of type string is deprecated
    107x in ContentTranslationSettingsTest::testSettingsUI from Drupal\Tests\content_translation\Functional
    31x in ContentTranslationSettingsTest::testAccountLanguageSettingsUI from Drupal\Tests\content_translation\Functional
    31x in ContentTranslationSettingsTest::testFieldTranslatableSettingsUI from Drupal\Tests\content_translation\Functional
    12x in ContentTranslationSettingsTest::testNonTranslatableTranslationSettingsUI from Drupal\Tests\content_translation\Functional

  3x: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
    3x in ContentTranslationSettingsTest::testSettingsUI from Drupal\Tests\content_translation\Functional

Comment #8

daffie CreditAttribution: daffie commented 4 October 2021 at 09:00

Status:

Needs review

» Needs work

The testbot is failing for PHP 8.1 rc2 and it should be failing for the test only patch.

Comment #9

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 4 October 2021 at 09:08

Status:

Needs work

» Needs review

@daffie this is just part of the fixes. The test only patch is to help people run it locally without the fix.

This patch is ready and will help us have green functional tests once we've sorted out behat/mink.

Comment #10

daffie CreditAttribution: daffie commented 4 October 2021 at 10:07

Status:

Needs review

» Needs work

The patch looks good to me, just one remark for me:

+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -76,6 +76,15 @@ public static function isDeprecationSkipped($message) {
+      // Skip Symfony deprecations for PHP 8.1 -  fixed by
+      // https://github.com/symfony/symfony/pull/42260.
+      '%Return type of Symfony\\\\Component\\\\.* should either be compatible with .*, or the #\[\\\\ReturnTypeWillChange\] attribute should be used to temporarily suppress the notice%',

This change should already be fixed in version v4.4.30 which is used in core now. See: https://github.com/symfony/symfony/pull/42260.

Comment #11

alexpott

he/they

English

🇪🇺🌍

CreditAttribution: alexpott at Acro Commerce, Thunder commented 4 October 2021 at 10:23

Status:

Needs work

» Needs review

File	Size
3-11-interdiff.txt	1.26 KB
3240456-11.patch	4.01 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass

@daffie great catch... nice one.

Comment #12

daffie CreditAttribution: daffie commented 4 October 2021 at 11:32

Status:

Needs review

» Reviewed & tested by the community

3 files were hidden/shown/deleted

File	Size
2-3-interdiff.txt	1.87 KB
3240456-3.test-only.patch	1.15 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass PHP 8.1 & sqlite-3.27 Build Successful
3240456-3.patch	4.31 KB
9.3.x: PHP 8.0 & MySQL 5.7 28,911 pass PHP 8.1 & sqlite-3.27 Build Successful

Adds the suppressions of 2 from dependency packages coming deprecation warnings and a test suppressing deprecation warning from PHP itself.
Looks good to me.

Comment #13

4 October 2021 at 14:43

catch committed f56323a on 9.3.x

Issue #3240456 by alexpott, andypost, daffie: Allow E_DEPRECATED...

Comment #14

catch

he/him

English

CreditAttribution: catch at Third and Grove commented 4 October 2021 at 14:43

Status:

Reviewed & tested by the community

» Fixed

Looks good.

Committed f56323a and pushed to 9.3.x. Thanks!

Comment #15

18 October 2021 at 14:44

Status:

Fixed

» Closed (fixed)

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

Allow E_DEPRECATED deprecations to be skipped so we can test on PHP 8.1

Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Parent issue

Thank you to these Drupal contributors

News items

Our community

Documentation

Drupal code base

Governance of community