Problem/Motivation

In #3060996: Fix The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead. we added a skipped deprecation that was triggered by behat/mink-browserkit-driver.

Since then behat/mink-browserkit-driver has released a new version that should remove the deprecation warning.

Proposed resolution

Remove the skipped deprecation.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#9 interdiff-2-9.txt837 bytessja112
#9 3135308-9.patch1.77 KBsja112
#2 3135308.patch971 byteslongwave
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Active » Needs review
FileSize
3135308.patch971 bytes

Status: Needs review » Needs work

The last submitted patch, 2: 3135308.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Needs review

Random fail.

Status: Needs review » Needs work

The last submitted patch, 2: 3135308.patch, failed testing. View results

Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 commented

Thanks for the patch.

1) I don't see anything wrong with the patch. I read the issue summary in #3060996: Fix The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead. and reviewed the commit https://git.drupalcode.org/project/drupal/commit/74e4b5d which added:

      // This deprecation comes from behat/mink-browserkit-driver when updating
      // symfony/browser-kit to 4.3+.
      'The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead.',

I compared this code with the code being removed in the patch and it's identical.

2) Patch applied cleanly to 9.0 and 9.1.

[mac:kristen:drupal-9.0.x-dev]$ patch -p1 < 3135308.patch
patching file core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
[mac:kristen:drupal-9.0.x-dev]$ cd91
[mac:kristen:drupal-9.1.x-dev]$ patch -p1 < 3135308.patch
patching file core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php

3) The issue summary says:

Since then behat/mink-browserkit-driver has released a new version that should remove the deprecation warning.

The patch from #3060996: Fix The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead. was committed 11 August 2019 at 17:25.

Versions of behat/mink-browserkit-driver are:

1.3.4 - 2020-03-11 09:49 UTC
1.3.3 - 2018-05-02 09:25 UTC

I'm unclear where this is fixed. I see the same getStatus() and getStatusCode() calls in both versions. I assume I don't understand something obvious here.

[mac:kristen:MinkBrowserKitDriver]$ git checkout v1.3.3
HEAD is now at 1b9a7ce... Merge pull request #127 from pamil/patch-1
[mac:kristen:MinkBrowserKitDriver]$ grep -r "getStatus()" .
./src/BrowserKitDriver.php:        return $this->getResponse()->getStatus();
[mac:kristen:MinkBrowserKitDriver]$ git checkout v1.3.4
Previous HEAD position was 1b9a7ce... Merge pull request #127 from pamil/patch-1
HEAD is now at e3b9084... Update the changelog
[mac:kristen:MinkBrowserKitDriver]$ grep -r "getStatus()" .
./src/BrowserKitDriver.php:            return $response->getStatus();

[mac:kristen:MinkBrowserKitDriver]$ git checkout v1.3.3
Previous HEAD position was e3b9084... Update the changelog
HEAD is now at 1b9a7ce... Merge pull request #127 from pamil/patch-1
[mac:kristen:MinkBrowserKitDriver]$ grep -r "getStatusCode()" .
./tests/Custom/BaseUrlTest.php:        $this->assertEquals(200, $session->getStatusCode());
./src/BrowserKitDriver.php:    public function getStatusCode()
[mac:kristen:MinkBrowserKitDriver]$ git checkout v1.3.4
Previous HEAD position was 1b9a7ce... Merge pull request #127 from pamil/patch-1
HEAD is now at e3b9084... Update the changelog
[mac:kristen:MinkBrowserKitDriver]$ grep -r "getStatusCode()" .
./tests/Custom/BaseUrlTest.php:        $this->assertEquals(200, $session->getStatusCode());
./src/BrowserKitDriver.php:    public function getStatusCode()
./src/BrowserKitDriver.php:        return $response->getStatusCode();
narendra.rajwar27’s picture

narendra.rajwar27
he/him
Primary language English
Location India
CreditAttribution: narendra.rajwar27 as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Unassigned » narendra.rajwar27
Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 commented

@narendra.rajwar27 If you aren't able to get to finish updating an issue within a few hours, it's best to unassign it so others might work on it. Thanks.

sja112’s picture

sja112 CreditAttribution: sja112 at Srijan | A Material+ Company for Drupal India Association commented
Assigned: narendra.rajwar27 » Unassigned
Status: Needs work » Needs review
FileSize
3135308-9.patch1.77 KB
interdiff-2-9.txt837 bytes

I have updated the patch to fix

'The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead.'

This deprecation is not added in the release. I am working on Drupal 9.1.x latest codebase.

#3060996: Fix The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead. 
Automatically closed - issue fixed for 2 weeks with no activity.

Please review.

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

@Kristen Pol: as always, thanks for the review! Tests will fail if deprecation notices still remain, as happened here - there is one case that was calling getStatus() directly and this caused the test to fail:

Remaining direct deprecation notices (1)

1x: The "Symfony\Component\BrowserKit\Response::getStatus()" method is deprecated since Symfony 4.3, use getStatusCode() instead.
1x in QuickEditImageControllerTest::testAccess from Drupal\Tests\image\Functional

If the dependency was calling the old method then more tests would have failed in the same way. If we look at the diff between 1.3.3 and 1.3.4 of behat/mink-browserkit-driver we can see that a fix was added to only call the old method if the new one doesn't exist:

https://github.com/minkphp/MinkBrowserKitDriver/compare/v1.3.3..v1.3.4#d...

@sja112: Thanks for fixing the test failure! Other than removing the deprecation skip, that is the only change that is required here, so this is now RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Version: 9.1.x-dev » 9.0.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed ef61d27660 to 9.1.x and 492710b7ab to 9.0.x. Thanks!

Backported to 9.0.x as this is a test-only fix.

  • alexpott committed ef61d27 on 9.1.x 
    Issue #3135308 by sja112, longwave, Kristen Pol: Fix 'The "Symfony\...

  • alexpott committed 492710b on 9.0.x 
    Issue #3135308 by sja112, longwave, Kristen Pol: Fix 'The "Symfony\...
Kristen Pol’s picture

Kristen Pol
she/her
Primary language English
Location Santa Cruz, CA, USA
CreditAttribution: Kristen Pol at Hook 42 commented

@longwave Thanks for the thoughtful explanation! And to @sja112 for the update! And to @alexpott for the commit! :)

Status: Fixed » Closed (fixed)

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