Problem/Motivation
I just discovered that there are two test files testing the D7 node deriver, MigrateNodeDeriverTest.php and NodeMigrateDeriverTest.php. The difference in name made me wonder if NodeMigrateDeriverTest was doing something unusual. It is not, it is just confusing have the two files. Lets merge them into one.
Tagging as a novice task.
Proposed resolution
Move d7\NodeMigrateDeriverTest::testBuilder to d7\MigrateNodeDeriverTest::testBuilder.
Delete the file NodeMigrateDeriverTest.php
Remaining tasks
Make a patch
Review commit
Comment | File | Size | Author |
---|---|---|---|
#8 | 3083196-8.patch | 4.08 KB | iyyappan.govind |
#5 | 3083196-5.patch | 4.08 KB | iyyappan.govind |
#2 | 3083196-2.patch | 4 KB | iyyappan.govind |
Comments
Comment #2
iyyappan.govindPlease review my patch. Thanks
Comment #3
iyyappan.govindComment #4
quietone CreditAttribution: quietone as a volunteer commentedThis needs a doc bloc. Could be something like, 'Tests that the field process is added to the pipeline.'
Comment #5
iyyappan.govindAdded the doc block. Please review it. Thanks
Comment #6
mikelutzAs long as we are in there, can we move this to
@group node
? I think it was copied over from contrib incorrectly.Comment #7
iyyappan.govindCan we add this as doc block?
Comment #8
iyyappan.govindUpdated the doc block. Please check it.
Comment #9
iyyappan.govindComment #10
quietone CreditAttribution: quietone as a volunteer commented@mikelutz, the @group tags for our tests are not consistent. Should we have an issue to tidy that up?
Comment #11
mikelutz@quietone They should always match the module machine_name, but I know we've moved tests from module to module and contrib to core etc, so I imagine there may be more that aren't right. Yes, we should have an issue to fix them all, but I'm still happy with fixing this one in this issue since we are in that file already.
Comment #12
iyyappan.govindHi mikelutz,
Thanks for reviewing the patch.
Comment #13
alexpottCommitted 9c171f8 and pushed to 8.8.x. Thanks!