Problem/Motivation

\Drupal\KernelTests\Core\Installer\InstallerLanguageTest::testInstallerTranslationFiles uses NULL as an array key. We can't do that anymore.

Steps to reproduce

Run \Drupal\KernelTests\Core\Installer\InstallerLanguageTest::testInstallerTranslationFiles

Proposed resolution

Move the NULL to be an array value so that we can continue to pass a NULL langcode into the function under test.

Remaining tasks

User interface changes

None

Introduced terminology

N/a

API changes

None

Data model changes

None

Release notes snippet

N/a

Issue fork drupal-3556721

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

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
Parent issue: » #3546535: [meta] Deal with NULL as array key/offset

alexpott opened merge request !13720

smustgrave’s picture

smustgrave commented

@alexpott where are you seeing this error? I ran it locally with 8.5 and didn't get any deprecation warnings or anything.

dcam’s picture

dcam commented
Status: Needs review » Needs work

I couldn't get it to fail locally with PHP 8.5 either. But afterward I realized that we have another potential conflict with the upcoming Drupal.Array.Array.LongLineDeclaration sniff.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review

This code does not fail the PHPCS rule. It is already checked by said rule and passes. The limit is not 80 chars it is 160 chars. Let's not mentally apply it. If you're going to set code to needs work because of a failing a PHPCS rule please check first.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Well this code is not checked BUT it does not fail the rule. The limit is 120 and not 80 so we're fine - so my point in #6 still stands.

smustgrave’s picture

smustgrave commented

What about #4

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@smustgrave this is definitely a problem. It's a null as an array key which this whole thing the deprecation is about.

From our own runner... https://git.drupalcode.org/issue/drupal-3556717/-/jobs/7213067#L3921 

---- Drupal\KernelTests\Core\Installer\InstallerLanguageTest ----


Status      Duration Info                                                                               
--------------------------------------------------------------------------------------------------------
[32mPass          1.098s testInstallerTranslationFiles                                                   
[0m[32mPass          0.781s testInstallerTranslationCache                                                   
[0m[31mFailure       2.876s *** Process execution output ***                                                
[0m    PHPUnit 11.5.42 by Sebastian Bergmann and contributors.
    
    Runtime:       PHP 8.5.0RC4
    Configuration: /builds/issue/drupal-3556717/core/phpunit.xml.dist
    
    [33;1mD[0m.                                                                  2 / 2 (100%)
    
    Time: 00:02.528, Memory: 8.00 MB
    
    [4mInstaller Language (Drupal\KernelTests\Core\Installer\InstallerLanguage)[0m
    [33m ⚠ [0mInstaller translation files
    [32m ✔ [0mInstaller translation cache
    
    1 test triggered 1 PHP deprecation:
    
    1) /builds/issue/drupal-3556717/core/tests/Drupal/KernelTests/Core/Installer/InstallerLanguageTest.php:27
    Using null as an array offset is deprecated, use an empty string instead
    
    [30;43mOK, but there were issues![0m
    [30;43mTests: 2[0m[30;43m, Assertions: 13[0m[30;43m, Deprecations: 1[0m[30;43m.[0m

[edit: swapped to a better link]

smustgrave’s picture

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

So relying on the pipeline which runs RC4. ddev still uses RC3 which is why I'm not seeing this.

  • catch committed c7018f4f on 11.3.x 
    Issue #3556721 by alexpott, dcam: Fix \Drupal\KernelTests\Core\Installer...

  • catch committed 2208fe3d on 11.x 
    Issue #3556721 by alexpott, dcam: Fix \Drupal\KernelTests\Core\Installer...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 11.3.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !13720

Status: Fixed » Closed (fixed)

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