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

CommentFileSizeAuthor
#9 3210898-4.patch6.08 KBquietone
#6 Screenshot 2021-05-05 at 12.27.05 PM.png44.6 KBbhumikavarshney
#4 3210898-4.patch6.08 KBquietone
#2 3210898-2.patch5.05 KBquietone

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Assigned: quietone » Unassigned
Issue summary: View changes
StatusFileSize
new 3210898-2.patch5.05 KB

Something like this seems much easier to understand.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented 
+++ b/core/modules/hal/tests/src/Unit/FieldNormalizerDenormalizeExceptionsUnitTest.php
@@ -27,4 +28,38 @@ public function testFieldNormalizerDenormalizeExceptions($context) {
+   * @coversDefaultClass \Drupal\hal\Normalizer\FieldItemNormalizer

@coversDefaultClass is only for class annotations, use @covers for method annotations.

quietone’s picture

quietone commented
Status: Active » Needs review
StatusFileSize
new 3210898-4.patch6.08 KB

@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.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bhumikavarshney’s picture

bhumikavarshney commented
StatusFileSize
new Screenshot 2021-05-05 at 12.27.05 PM.png44.6 KB

Hi @quietone,
The above patch works for me.
3 files are deleted and 1 new file is created.
Thanks!!
RTBC+1

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Combine tests using NormalizerDenormalizeExceptionsUnitTestBas » Combine tests using NormalizerDenormalizeExceptionsUnitTestBase
Status: Needs review » Reviewed & tested by the community

This makes sense to me, it is much simpler to understand these tests now.

The last submitted patch, 2: 3210898-2.patch, failed testing. View results

quietone’s picture

quietone commented
StatusFileSize
new 3210898-4.patch6.08 KB

I 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.

  • catch committed 00b7574 on 9.3.x 
    Issue #3210898 by quietone, BhumikaVarshney: Combine tests using...

  • catch committed 22a5978 on 9.2.x 
    Issue #3210898 by quietone, BhumikaVarshney: Combine tests using...
catch’s picture

catch
he/him
Primary language English
commented
Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/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.

Status: Fixed » Closed (fixed)

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