Problem/Motivation

Discovered whilst working on #3219959-63: Update standard profile so Olivero is the default theme

There are still a few protected $dumpHeaders = TRUE; floating around in the Tests.
Seems to me this is a leftover from the ages of \Drupal\simpletest\WebTestBase.

By 9.0.0 we went with Headers all the time in \Drupal\Tests\BrowserHtmlDebugTrait

Proposed resolution

Remove all protected $dumpHeaders = TRUE; and send it the way of the dodo.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 interdiff_2152-2155.txt2.48 KBspokje

Issue fork drupal-3276839

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

Spokje created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
Related issues: +#3219959: Update standard profile so Olivero is the default theme

Spokje opened merge request !2152

spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Yes, cruft. In the 10 branch PHPStan also spotted some orphans here, and #3274474: Fix 'Access to an undefined property' PHPStan L0 errors is removing them.

mondrake’s picture

mondrake
Location 🇮🇹
commented

btw a D10 patch will require some phpstan baseline cleanup.

spokje’s picture

spokje
Primary language Dutch
commented

Thanks @mondrake for pointing out the D10 phpstan baseline thingy.

I'll leave it up to committers discretion if #3274474: Fix 'Access to an undefined property' PHPStan L0 errors should go in first (which would make this issue obsolete), or we need a D10 patch with the removal of the errors in core/phpstan-baseline.neon and get both patches in, which would make the mentioned issue (a tiny bit) smaller.

mondrake’s picture

mondrake
Location 🇮🇹
commented

@spokje this one is more complete re the scope it has. The other would only remove a few. Also the other may be D10 only. So I think a D10 patch version of the MR here would do.

Spokje opened merge request !2155

spokje’s picture

spokje
Primary language Dutch
commented
Status: Reviewed & tested by the community » Needs review

@mondrake: Sense = made.

The D10 version and core/phpstan-baseline.neon pointed out a few I've missed in my original D9.4 MR.

So back to NR:

MR!2152: 9.4.x-dev
MR!2155: 10.0.x-dev

Attached a interdiff between the 2 MR diffs. It looks a bit wonky for core/modules/hal/tests/src/Functional/page_cache/PageCacheTest.php since that's deleted in 10.0.x-dev. In 9.4.x-dev it's actually removing the dumpHeaders property: See here

spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new interdiff_2152-2155.txt2.48 KB
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community

Thanks 🙏 👍

quietone’s picture

quietone commented
Version: 9.4.x-dev » 10.0.x-dev
Status: Reviewed & tested by the community » Needs work

Tests are no longer running on this, it looks like it needs a rebase.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community

Rebased both MRs

spokje’s picture

spokje
Primary language Dutch
commented
Title: Remove leftover dumpHeaders property » [PP-1] Remove leftover dumpHeaders property
Status: Reviewed & tested by the community » Postponed
Related issues: +#3280050: core/phpstan-baseline.neon out of sync

Postponing on new (correct) PHPStan baseline: #3278782: PHPStan baseline is out of sync

spokje’s picture

spokje
Primary language Dutch
commented
Related issues: +#3278782: PHPStan baseline is out of sync
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-1] Remove leftover dumpHeaders property » Remove leftover dumpHeaders property
Status: Postponed » Reviewed & tested by the community
Issue tags: +PHPStan-0

Rerolled the D10 patch after baseline was fixed. Back to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed fdfde8e and pushed to 10.0.x. Thanks!
Committed 1f1cdd2 and pushed to 9.5.x. Thanks!
Committed 87e6164 and pushed to 9.4.x. Thanks!

  • alexpott committed fdfde8e on 10.0.x 
    Issue #3276839 by Spokje, mondrake: Remove leftover dumpHeaders property

  • alexpott committed 1f1cdd2 on 9.5.x 
    Issue #3276839 by Spokje, mondrake: Remove leftover dumpHeaders property

  • alexpott committed 87e6164 on 9.4.x 
    Issue #3276839 by Spokje, mondrake: Remove leftover dumpHeaders property...

Spokje closed merge request !2152

Spokje closed merge request !2155

Status: Fixed » Closed (fixed)

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