Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
This is a follow up to #3134459: Remove @group legacy from migrate tests which removed '@group legacy' from migration tests. This is to do the same for all tests.
They can be found with `grep -r '@group legacy' core` which currently returns 42 lines.
Not sure what component to use so selected 'other'.
Proposed resolution
Remove '@group legacy' from tests where it is no longer needed.
Remaining tasks
Make a patch
Review
Comment | File | Size | Author |
---|---|---|---|
#21 | 3136389-21.patch | 8.13 KB | Suresh Prabhu Parkala |
#20 | 3136389-20.patch | 8.91 KB | Suresh Prabhu Parkala |
#14 | interdiff-12-14.txt | 532 bytes | Hardik_Patel_12 |
#14 | 3136389-14.patch | 8.91 KB | Hardik_Patel_12 |
Comments
Comment #2
jyotimishra-developer CreditAttribution: jyotimishra-developer commentedComment #3
shimpyRemoved @group legacy tags from test
Comment #4
longwaveWe can't just remove it from all places, we do need the legacy group when we are testing deprecations.
#3135302: Remove Symfony 3 legacy test from ReverseProxyMiddlewareTest covers one of these as well, maybe this should be a meta to review each of these and have child issues for any we find that need to be removed - as in that case some may need more than just the group removing? Not sure.
Comment #5
jyotimishra-developer CreditAttribution: jyotimishra-developer commentedHi @shimpy, I have assigned this issue to me and was working on it..i feel its unethical.
But anyways..no issues.
Comment #6
jyotimishra-developer CreditAttribution: jyotimishra-developer commentedRemoved @group legacy tags from test and corrected errors in patch #3, found in dispatcher result.
Comment #7
jyotimishra-developer CreditAttribution: jyotimishra-developer commentedComment #8
jyotimishra-developer CreditAttribution: jyotimishra-developer commentedComment #9
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedComment #10
longwaveThanks for working on this.
This whole comment can be removed now, because getLabelCallback() has been removed too.
This comment block can be removed too.
This whole test can be removed, but let's leave this for #3135302: Remove Symfony 3 legacy test from ReverseProxyMiddlewareTest
This shouldn't be removed, please read the comment above.
Comment #11
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedComment #12
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commented@longwave thanks for your review , i have covered point no 1 , 2 and 4 in this patch as mentioned in #10. Point number 3 is leave for #3135302: Remove Symfony 3 legacy test from ReverseProxyMiddlewareTest.
Comment #13
longwaveThanks! Two tiny nitpicks then this is good to go:
There is an empty comment line at the end of the comment that can now be removed.
There is an empty comment line at the end of the comment that can now be removed.
Comment #14
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedEmpty comment line are removed from
core/modules/user/tests/src/Unit/Views/Argument/RolesRidTest.php
andcore/tests/Drupal/Tests/Core/Asset/AssetResolverTest.php
files.Comment #15
longwaveThanks, this looks perfect now!
Comment #16
xjmSaving reviewer and reporter credits.
Comment #17
jyotimishra-developer CreditAttribution: jyotimishra-developer at Srijan | A Material+ Company for Drupal India Association commentedComment #18
longwaveNeeds rerolling.
Comment #19
naresh_bavaskarComment #20
Suresh Prabhu Parkala CreditAttribution: Suresh Prabhu Parkala at Specbee commentedRe-rolled patch please review.
Comment #21
Suresh Prabhu Parkala CreditAttribution: Suresh Prabhu Parkala at Specbee commentedPlease review!
Comment #22
longwaveThanks for rerolling.
#21 looks good, same as #14 with ReverseProxyMiddlewareTest removed as that was committed separately.
Comment #24
longwaveRandom fail in WidgetUploadTest, back to RTBC.
Comment #25
xjmComment #26
alexpottCommitted and pushed 409af2de16 to 9.1.x and 00798cc725 to 9.0.x. Thanks!
This is an important fix to ensure we're not triggering deprecated code. Nice work everyone.
Backported to 9.0.x as this is a test-only fix.