Problem/Motivation

In core/tests/Drupal/Tests/Core/Routing/RequestFormatRouteFilterTest.php file $route variable is initialized in multiple function but never used .

Proposed resolution

Remove unused $route variables.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Hardik_Patel_12 created an issue. See original summary.

siddhant.bhosale’s picture

Assigned: Unassigned » siddhant.bhosale
siddhant.bhosale’s picture

Assigned: siddhant.bhosale » Unassigned
Status: Needs review » Reviewed & tested by the community

Hi, the patch applies cleanly. There are no test errors. Looks good to be merged.

apaderno’s picture

Status: Reviewed & tested by the community » Needs work

We need to check if it's just a matter of initialized variables that aren't used, or it's a bigger issue, for example code that has been wrongly removed.
We need to verify in which commit the variables have been introduced, and which commit removed the code using those variables.

paulocs’s picture

Status: Needs work » Needs review

So lets go per parts:

The lines 39, 41 and 75 were added in the issue #2481453: Implement query parameter based content negotiation as alternative to extensions and as I see, it is no problem to remove them.

The line 106 were added in the issue #2854560: \Drupal\Core\Routing\RequestFormatRouteFilter::filter() is too HTML-centric and it is no problem to remove too.

apaderno’s picture

Status: Needs review » Reviewed & tested by the community

The first patch added all the testFilter() code; the second patch moved part of the code in a data provider.

In neither the cases, the $route variable was used. It's not the case of removed code that should not have been removed, then. The patch is correctly fixing the code by removing the unused variables.

alexpott’s picture

Version: 9.1.x-dev » 8.9.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 87713ec20e to 9.1.x and 5679afdce0 to 9.0.x and 464b755f85 to 8.9.x. Thanks!

FILE: ...tests/Drupal/Tests/Core/Routing/RequestFormatRouteFilterTest.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
 38 | ERROR | [x] Expected 1 space after "="; 2 found
 40 | ERROR | [x] Expected 1 space after "="; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

Fixed on commit.

Backported to 8.9.x to keep the tests aligned.

  • alexpott committed 03e067b on 9.1.x
    Issue #3158276 by Hardik_Patel_12, kiamlaluno, paulocs: Remove local...

  • alexpott committed 5679afd on 9.0.x
    Issue #3158276 by Hardik_Patel_12, kiamlaluno, paulocs: Remove local...

  • alexpott committed 464b755 on 8.9.x
    Issue #3158276 by Hardik_Patel_12, kiamlaluno, paulocs: Remove local...

Status: Fixed » Closed (fixed)

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