Problem/Motivation
In core/tests/Drupal/Tests/Core/Render/BubbleableMetadataTest.php testMergeAttachmentsLibraryMerging() initializes $settings_two['moduleName']['thingiesOnPage']['id1'], which is then never used.
Proposed resolution
Remove $settings_two['moduleName']['thingiesOnPage']['id1'].
// adds the exact same settings twice and hence tests idempotency, the
// second adds *almost* the same settings twice: the second time, some
// values are altered, and some key-value pairs are added.
- $settings_two['moduleName']['thingiesOnPage']['id1'] = [];
$this->assertSame($settings_one, $merged['drupalSettings']['commonTestRealWorldIdentical']);
$expected_settings_two = $settings_two_a;
$expected_settings_two['moduleName']['ui'][0] = 'button D';
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | drupal-remove-unused-variable-3158286-6.patch | 1.06 KB | avpaderno |
| unsused-variable-removed.patch | 880 bytes | hardik_patel_12 |
Comments
Comment #2
avpadernoReading the comment before the assignment, I wonder if there is simply a typo in the variable name.
Maybe the line should simply be the following one.
Comment #3
avpadernoThis is harder to track.
The code in core/tests/Drupal/Tests/Core/Render/BubbleableMetadataTest.php that includes the
$settings_two['moduleName']['thingiesOnPage']['id1'] = [];line (truly,$settings_two['moduleName']['thingiesOnPage']['id1'] = array();) has been added in #2483433: Optimize CacheableMetadata::merge() + BubbleableMetadata::merge(), but the patch in that issue simply moved existing code from theMergeAttachmentsTestclass to theBubbleableMetadataTestclass where it is now.I will try to understand what code was used before that patch, after I find a commit that still contains the
MergeAttachmentsTestclass.Comment #4
avpaderno#2382557: Change JS settings into a separate asset type moved the
$settings_two['moduleName']['thingiesOnPage']['id1'] = array();line from core/modules/system/src/Tests/Common/JavaScriptTest.php to core/modules/system/src/Tests/Common/MergeAttachmentsTest.php.(I apologize for posting multiple comments, but it's rather long to find out when a single line has been added in a file with
git log --full-history, especially when that line keeps being moved from file to file.)Comment #5
avpadernoFinally, this is the code used in commit 8c68491896ce0a4a52dce9f8de94d23623c70fef.
$settings_twois changed right before comparing it with$parsed_settings['commonTestRealWorldAlmostIdentical']. That doesn't happen with the currently used code.Comparing the code from that commit and the currently used code, I conclude the currently used code should be the following one.
Comment #6
avpadernoComment #7
longwave@kiamlaluno++ excellent git analysis, agree with all your findings here that this is a typo/refactoring error rather than simply an unused variable.
Comment #9
catchNice bit of archaeology.
Committed 836157c and pushed to 9.1.x. Thanks!