The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
In core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php, $assert_session variable never used.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | remove_assert_section-3173891-5.patch | 737 bytes | meena.bisht |
| #2 | assertsession.png | 649.9 KB | shetpooja04 |
| #2 | 3173891-2.patch | 766 bytes | shetpooja04 |
Comments
Comment #2
shetpooja04 commentedCommit ID: 5d95f2e1
Link: https://git.drupalcode.org/project/drupal/-/commit/5d95f2e177facb2895153ae68619e042f72b93a2
File: core/modules/system/tests/src/Functional/UpdateSystem/UpdateScriptTest.php Line: 417
For Issue: https://www.drupal.org/project/drupal/issues/3136668 the variable is added but not used
Please review
Comment #3
meena.bisht commentedComment #4
meena.bisht commentedAs , unable to find use of $this->assertSession();. So putting it back to needs work.
Comment #5
meena.bisht commentedUploading patch , removing $this->assertSession();.
Comment #6
ranjith_kumar_k_u commentedThe above patch looks good,the variable "$assert_session" is not used anywhere in the testOrphanedSchemaEntries() function ,so there is no need of "$this->assertSession();" also.RTBC
Comment #8
sulfikar_s commentedThe above patch applied cleanly on 9.2.x-dev. The variable "$assert_session" is not used under the function testOrphanedSchemaEntries(). So removing the line is the better option. Appreciate your research !.
Changing the status to RTBC.
Comment #9
sulfikar_s commentedComment #13
xjmThanks @shetpooja04 for documenting the issue that added the variable and showing that it was unused from the start. It's probable that the author planned to use it for assertions but the test did not end up requiring it. We definitely need that information when we commit these.
@meena.bisht's fix is also correct here.
$this->assertSession()doesn't do anything on its own; it's just a getter for a new WebAssert session. Reference: https://api.drupal.org/api/drupal/vendor%21behat%21mink%21src%21Mink.php...@sulfikar_s and @ranjith_kumar_k_u, in the future, you can provide additional help out by verifying the information about when the variable was added and how it ended up unused, like the info that @shetpooja04 provided and anything else you can discover. (It won't always be in a single issue). That's information we use to make sure these unused variables aren't hiding bugs. It's important not just that it's unused, but why.
As a removal of dead code and test fix, this issue is backportable to the production branch. So, committed to 9.2.x, and backported to 9.1.x and 9.0.x (to keep the branches closer together). I did not backport to 8.9.x at this time but will queue an 8.9.x test run to see if the patch can be backported there as well.
Thanks!
Comment #14
catch8.9.x is security only now, so moving this to fixed against 9.0.x