Problem/Motivation

Move the method MigrateOrderItemTypeTest::assertOrderItemType to CommerceMigrateTestTrait.
Decide if more assertions can be added. If so, the do it.

Proposed resolution

  1. Move the method
  2. Add assertions, if needed.

Remaining tasks

All the above

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#6 move_assertOrderITemType_to_TestTrait-2905482-6.patch2.47 KBkarthikkumarbodu
#4 move_assertOrderITemType_to_TestTrait-2905482-4.patch2.4 KBkarthikkumarbodu
#2 move_assertOrderITemType_to_TestTrait-2905482-2.patch2.2 KBkarthikkumarbodu

Comments

quietone created an issue. See original summary.

karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Active » Needs review
StatusFileSize
new move_assertOrderITemType_to_TestTrait-2905482-2.patch2.2 KB

Status: Needs review » Needs work

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

karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Needs work » Needs review
StatusFileSize
new move_assertOrderITemType_to_TestTrait-2905482-4.patch2.4 KB
quietone’s picture

quietone commented
Status: Needs review » Needs work 
  1. +++ b/modules/commerce/tests/src/Kernel/Migrate/d7/MigrateOrderItemTypeTest.php
@@ -2,7 +2,8 @@
+

    Remove extra line 

  2. +++ b/tests/src/Kernel/CommerceMigrateTestTrait.php
@@ -425,4 +426,18 @@ trait CommerceMigrateTestTrait {
+  public function assertOrderItemType($id, $expected_label) {

    The methods in CommerceMigrateTestTrait are ordered alphabetically. Can we move this up to be in alpha order? I fully accept this is solely about my sense of order and isn't strictly necessary.

karthikkumarbodu’s picture

karthikkumarbodu commented
Status: Needs work » Needs review
StatusFileSize
new move_assertOrderITemType_to_TestTrait-2905482-6.patch2.47 KB
heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Reviewed & tested by the community

@karthikkumarbodu for the future, could you add an interdiff when posting updated patches? https://www.drupal.org/documentation/git/interdiff

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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