Problem/Motivation

During kernel tests configuration is not installed for modules unless you call ->installConfig(). This can cause deprecations in PHP 8.1 because code that expects the code to exist ends up passing a NULL to a string function.

Steps to reproduce

See test runs in #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves) especially https://www.drupal.org/pift-ci-job/2120084

Proposed resolution

Install the configuration properly.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3239292-2.patch4.72 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
Issue tags: +PHP 8.1
Parent issue: » #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves)
FileSize
3239292-2.patch4.72 KB
daffie’s picture

daffie CreditAttribution: daffie commented

Is there a testbot run with the failures, so that I can see that the added code is necessary? Or some other way that I can make that determination?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Issue summary: View changes

@daffie it's not that simple - we're not passing on PHP 8.1 yet - hence the need to fix this. The tests fixed here fails in https://www.drupal.org/pift-ci-job/2120084 but there is plenty of other fails there too. All of the tests fixed here will have commits on that branch making it possible to find a test run prior to it being fixed. But there is a lot of noise.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

I've just kicked off a test using PHP 8.1 and 9.3.x HEAD - https://www.drupal.org/pift-ci-job/2189809 - we should see the fails there - hopefully enough things having been fixed in HEAD to make that useful.

daffie’s picture

daffie CreditAttribution: daffie commented

@alexpott: I have search the testbot results the IS/comment #4 and for AggregatorItemViewsFieldAccessTest I found the following errors:

Drupal\Tests\aggregator\Kernel\Views\AggregatorItemViewsFieldAccessTest

fail: [Other] Line 0 of sites/default/files/simpletest/phpunit-19.xml:
PHPUnit Test failed to complete; Error: PHPUnit 9.5.6 by Sebastian Bergmann and contributors.

Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

Testing Drupal\Tests\aggregator\Kernel\Views\AggregatorItemViewsFieldAccessTest
.                                                                   1 / 1 (100%)

Time: 00:02.617, Memory: 4.00 MB

OK (1 test, 17 assertions)

Unsilenced deprecation notices (5)

  4x: htmlspecialchars(): Passing null to parameter #1 ($string) of type string is deprecated
    4x in AggregatorItemViewsFieldAccessTest::testAggregatorItemFields from Drupal\Tests\aggregator\Kernel\Views

  1x: preg_split(): Passing null to parameter #2 ($subject) of type string is deprecated
    1x in AggregatorItemViewsFieldAccessTest::testAggregatorItemFields from Drupal\Tests\aggregator\Kernel\Views

Can you say that those errors will be gone when you do a testbot run with the patch from this issue is applied? I am not sure what kind of errors I should see? I would very much like to confirm that the patch fixes the problem and that we got all those missing configurations added.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

@daffie it's really hard because we have deprecations on top of deprecations happening here. Personally I think that installing the config of a module when it's installed in a kernel test should not be optional. It's an optimisation which can lead to unexpected and incorrect testing as it is never true that a module is installed without its simple configuration. Sure config entities can be deleted but simple config is a singleton and should be able to be depended upon.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

What we can confirm is that with these changes these tests are green when all the other fixes for PHP 8.1 are applied. That's why the meta exists.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs review » Reviewed & tested by the community

++ to have config of tested modules installed (more consistent testing)

  • catch committed e420029 on 9.3.x 
    Issue #3239292 by alexpott: Missing configuration in KernelTests causes...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed e420029 and pushed to 9.3.x. Thanks!

Status: Fixed » Closed (fixed)

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