Problem/Motivation

AssertLegacyTrait::assertHeader() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseHeaderEquals() instead. See https://www.drupal.org/node/3129738

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#18 3139439-D89-and-D90-18.patch11.59 KBBunty Badgujar
#18 3139439-D88-16.patch11.95 KBBunty Badgujar
#16 3139439-16.patch11.95 KBBunty Badgujar
#16 interdiff_13-16.txt788 bytesBunty Badgujar
#13 3139439-13.patch12.91 KBBunty Badgujar
#6 3139439-6.patch14.49 KBmondrake
#6 interdiff_4-6.txt803 bytesmondrake
#4 3139439-4.patch14.49 KBmondrake
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mondrake created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented
Issue tags: +Deprecated assertions

Adding the "Deprecated assertions" tag to add this into the kanban board https://contribkanban.com/board/Deprecatedassertions

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Assigned: Unassigned » mondrake

on this.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Assigned: mondrake » Unassigned
Status: Active » Needs review
FileSize
3139439-4.patch14.49 KB

This seems simple. Replaced calls, removed deprecation silencer, added deprecation test.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work 
+++ b/core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php
@@ -249,6 +249,19 @@ public function testAssertResponse() {
+  public function testAssertResponse() {

You tried to create a second method with the same name.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff_4-6.txt803 bytes
3139439-6.patch14.49 KB

Oops. Thank you.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Looks good to me!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: Replace usages of AssertLegacyTrait::assertHeader, that is deprecated » Replace usages of deprecatd AssertLegacyTrait::assertHeader()
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: Replace usages of deprecatd AssertLegacyTrait::assertHeader() » Replace usages of deprecated AssertLegacyTrait::assertHeader()

Title cleanup fail.

  • xjm committed a6c0ada on 9.1.x 
    Issue #3139439 by mondrake, daffie: Replace usages of deprecated...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Version: 9.1.x-dev » 8.8.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Perfect, thanks! All remaining references are either the method itself or its legacy tests:

[ibnsina:maintainer | Sun 16:38:46] $ grep -r "assertHeader(" * | grep -v "vendor"
core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php:   * Tests legacy assertHeader().
core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php:   * @expectedDeprecation AssertLegacyTrait::assertHeader() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseHeaderEquals() instead. See https://www.drupal.org/node/3129738
core/tests/Drupal/FunctionalTests/BrowserTestBaseTest.php:    $this->assertHeader('X-Drupal-Cache-Tags', 'http_response rendered');
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:  protected function assertHeader($name, $value) {
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:    @trigger_error('AssertLegacyTrait::assertHeader() is deprecated in drupal:8.2.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseHeaderEquals() instead. See https://www.drupal.org/node/3129738', E_USER_DEPRECATED);

PTBP for a backport that doesn't un-silence the deprecation. Thanks!

Bunty Badgujar’s picture

Bunty Badgujar CreditAttribution: Bunty Badgujar commented
Assigned: Unassigned » Bunty Badgujar
Bunty Badgujar’s picture

Bunty Badgujar CreditAttribution: Bunty Badgujar commented
Status: Patch (to be ported) » Needs review
FileSize
3139439-13.patch12.91 KB

#6 ported to 8.8.x

Status: Needs review » Needs work

The last submitted patch, 13: 3139439-13.patch, failed testing. View results

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

@Bunty Badgujar we have to remove the deprecation test, too - see #3139403-26: Replace usages of deprecated AssertLegacyTrait::assertElement(Not)Present()

Bunty Badgujar’s picture

Bunty Badgujar CreditAttribution: Bunty Badgujar commented
Status: Needs work » Needs review
FileSize
interdiff_13-16.txt788 bytes
3139439-16.patch11.95 KB

@mondrake, Thanks for this information. I thought deprecation test handle similar to 9.x.

Removed deprecation test.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

Looks ike we need a separate patch for D8.9 and D9.0 - general suggestion: when backporting, go from the highest to the lowest... so that the flows of commit can be 9.0, then 8.9, then 8.8. I doubt core committers will be willing to commit something that leaves holes in between... :)

Bunty Badgujar’s picture

Bunty Badgujar CreditAttribution: Bunty Badgujar commented
FileSize
3139439-D88-16.patch11.95 KB
3139439-D89-and-D90-18.patch11.59 KB

Adding patch for 8.9 and 9.0. Re-adding #16 just to combine all patch in single comment.

mondrake’s picture

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

LGTM

xjm’s picture

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

These look good. There is one other place the old method is used in D8:

core/modules/views_ui/src/Tests/UITestBase.php:      $this->assertHeader('Content-Type', 'application/json');

Should we fix that too?

Meanwhile I'll commit the 9.0.x patch but just waiting on a retest.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: Replace usages of deprecated AssertLegacyTrait::assertHeader() » [backport] Replace usages of deprecated AssertLegacyTrait::assertHeader()

Forgot to add a note in the title.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

#20 that's a Simpletest base class, so no, it should not be touched.

xjm’s picture

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

Ah good call, thanks @mondrake. I did not actually expect them to still be around in 8.9! I guess it's because it was a base class and we didn't want to rm -rf those since they can be considered API.

Bunty Badgujar’s picture

Bunty Badgujar CreditAttribution: Bunty Badgujar at Srijan | A Material+ Company for Drupal India Association commented

  • xjm committed 736a36c on 9.0.x 
    Issue #3139439 by Bunty Badgujar, mondrake, xjm, daffie: Replace usages...

  • xjm committed e239d1d on 8.9.x 
    Issue #3139439 by Bunty Badgujar, mondrake, xjm, daffie: Replace usages...

  • xjm committed dc50101 on 8.8.x 
    Issue #3139439 by Bunty Badgujar, mondrake, xjm, daffie: Replace usages...
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed

Committed the backports to 9.0.x, 8.9.x, and 8.8.x. Thanks all!

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Title: [backport] Replace usages of deprecated AssertLegacyTrait::assertHeader() » Replace usages of deprecated AssertLegacyTrait::assertHeader()
Status: Fixed » Reviewed & tested by the community
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Fixed

#28 did not take for some reason.

Status: Fixed » Closed (fixed)

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