Problem/Motivation

As part of #2796393: Migration process plugin not working with multiple source IDs I have been trying to add a test of the Migration process plugin, but have found the existing tests hard to understand and follow, mainly due to repeated code.

Proposed resolution

The existing tests can be simplified by adding a setUp() method and helper method(s).
The data provider can be tidied by replacing the comments with array keys.

Remaining tasks

Extract common code to setUp() and helper method(s).
Tidy data provider.

User interface changes

-

API changes

-

Data model changes

-

CommentFileSizeAuthor
#7 interdiff.txt943 bytesboaloysius
#7 2867340-7.patch10.8 KBboaloysius
#3 2867340-3.patch10.8 KBjofitz
#3 interdiff-2-3.txt2.43 KBjofitz
#2 2867340-2.patch8.66 KBjofitz

Comments

Jo Fitzgerald created an issue. See original summary.

jofitz’s picture

jofitz
he/him
commented
Assigned: jofitz » Unassigned
Status: Active » Needs review
StatusFileSize
new 2867340-2.patch8.66 KB

Extracted common code to setUp() and helper method getMigration().

jofitz’s picture

jofitz
he/him
commented
Issue summary: View changes
StatusFileSize
new interdiff-2-3.txt2.43 KB
new 2867340-3.patch10.8 KB

While I'm at it I'll tidy up the data provider too.

quietone’s picture

quietone commented

@Jo Fitzgerald, this look really good. A definite improvement. I'd RTBC but will defer to someone more with more knowledge about tests.

heddn’s picture

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

A lot easier to follow. Thanks for the improvemens.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/migrate/tests/src/Unit/process/MigrationTest.php
@@ -152,49 +171,29 @@ public function testSuccessfulLookup($source_id_values, $destination_id_values,
-      // Test data for scalar to scalar.
...
+      'scaler_to_scaler' => [
...
-      // Test data for scalar to array.
...
+      'scaler_to_array' => [
...
-      // Test data for array to scalar.
...
+      'array_to_scaler' => [

Somewhat minor but the new code uses 'scaler' when it should say 'scalar'.

boaloysius’s picture

boaloysius commented
StatusFileSize
new 2867340-7.patch10.8 KB
new interdiff.txt943 bytes

Made spelling corrections as stated in #6

boaloysius’s picture

boaloysius commented
Status: Needs work » Needs review
heddn’s picture

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

Back to RTBC again. (assuming tests pass on a docs only modification).

jofitz’s picture

jofitz
he/him
commented

@Cottser++
@JF--

Thanks for spotting that!

  • Gábor Hojtsy committed c4cbc76 on 8.3.x 
    Issue #2867340 by Jo Fitzgerald, boaloysius, Cottser: Improve...

  • Gábor Hojtsy committed d66aaae on 8.4.x 
    Issue #2867340 by Jo Fitzgerald, boaloysius, Cottser: Improve...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: +Baltimore2017

Looks good, resolved the concern from Cottser. Thanks all!

Status: Fixed » Closed (fixed)

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