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
While working on #3123060: Enable 'Drupal.Commenting.DocComment.ShortSingleLine' coding standard I noticed that \Drupal\Tests\hal\Unit\NormalizerDenormalizeExceptionsUnitTestBase is a data provider for two tests, both in separate files, \Drupal\Tests\hal\Unit\FieldNormalizerDenormalizeExceptionsUnitTest and \Drupal\Tests\hal\Unit\FieldItemNormalizerDenormalizeExceptionsUnitTest. They can be combined into one file.
Steps to reproduce
Proposed resolution
Remaining tasks
Review
Commit
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#9 | 3210898-4.patch | 6.08 KB | quietone |
#6 | Screenshot 2021-05-05 at 12.27.05 PM.png | 44.6 KB | BhumikaVarshney |
#4 | 3210898-4.patch | 6.08 KB | quietone |
#2 | 3210898-2.patch | 5.05 KB | quietone |
Comments
Comment #2
quietone CreditAttribution: quietone as a volunteer commentedSomething like this seems much easier to understand.
Comment #3
longwave@coversDefaultClass is only for class annotations, use @covers for method annotations.
Comment #4
quietone CreditAttribution: quietone as a volunteer commented@longwave, thanks.
I also didn't change the extends from the old base class to UnitTestCase. I then decided to change the file name as well to remove Unit' since this is in the Drupal\Tests\hal\Unit namespace. Because of the name change the interdiff was useless so I have omitted it and the diff wasn't any better. So just a new patch.
Comment #6
BhumikaVarshney CreditAttribution: BhumikaVarshney as a volunteer and at OpenSense Labs commentedHi @quietone,
The above patch works for me.
3 files are deleted and 1 new file is created.
Thanks!!
RTBC+1
Comment #7
longwaveThis makes sense to me, it is much simpler to understand these tests now.
Comment #9
quietone CreditAttribution: quietone as a volunteer commentedI notice what is probably a random failure on the 9.3.x patch. I've decided to re-upload the patch so testing is against 9.3.x.
Comment #12
catchCommitted/pushed to 9.3.x and cherry-picked to 9.2.x. Messed up the commit credit in the commit, but adding @longwave in the issue.