Problem/Motivation

We have a few migrations such as the d6_user migration which set a yml key of "no_stub" but in \Drupal\migrate\Plugin\migrate\process\Migration we use the key "no stub".

Proposed resolution

Rename to be consistent and add some tests.

Remaining tasks

Decide on whether to use "no_stub" or "no stub".

User interface changes

n/a

API changes

n/a

CommentFileSizeAuthor
#8 interdiff.txt947 bytesbenjy
#8 2337749-8.patch3.57 KBbenjy
#6 interdiff.txt780 bytesbenjy
#6 2337749-5.patch3.46 KBbenjy
#4 2337749-4.patch3.52 KBbenjy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjy’s picture

benjy CreditAttribution: benjy commented

My preference would be "no_stub". I think keys with spaces is a Drupal thing and I saw somewhere else in the issue queue we were trying to do away with them for permissions as well.

benjy’s picture

benjy CreditAttribution: benjy commented
Related issues: +#2337749: no_stub is never used
benjy’s picture

benjy CreditAttribution: benjy commented
Related issues: -#2337749: no_stub is never used +#2306049: D6->D8 node migration - Handle nodes with format = 0 ?
benjy’s picture

benjy CreditAttribution: benjy commented
Status: Active » Needs review
FileSize
2337749-4.patch3.52 KB

Patch attached renames to no_stub and adds a test.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Needs work 
+    $storage = $this->getMockBuilder('Drupal\Core\Entity\EntityStorageInterface')
+      ->disableOriginalConstructor()
+      ->getMock();

just getMock('interface') is enough -- interfaces dont have anything to disable (which is why we prefer them in typehint to classes).

benjy’s picture

benjy CreditAttribution: benjy commented
Status: Needs work » Needs review
FileSize
2337749-5.patch3.46 KB
interdiff.txt780 bytes

Thanks for the review :)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/modules/migrate/tests/src/Unit/process/MigrationTest.php
@@ -0,0 +1,67 @@
+/**
+ * Tests the flatten plugin.
+ *
+ * @group migrate
+ */
+class MigrationTest extends MigrateProcessTestCase {

Let's use @coversDefaultClass and @covers so that we don't have to write a wrong description ;) Note: I took some of the code here over to https://www.drupal.org/node/2321609#comment-9145537

benjy’s picture

benjy CreditAttribution: benjy commented
FileSize
2337749-8.patch3.57 KB
interdiff.txt947 bytes

Thanks for the review.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/migrate/tests/src/Unit/process/MigrationTest.php
@@ -0,0 +1,72 @@
+ * @coversDefaultClass \Drupal\migrate\Plugin\migrate\process\Migration
...
+class MigrationTest extends MigrateProcessTestCase {

OOH we don't test the entity itself, got it!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 04fdb83 and pushed to 8.0.x. Thanks!

diff --git a/core/modules/migrate/tests/src/Unit/process/MigrateProcessTestCase.php b/core/modules/migrate/tests/src/Unit/process/MigrateProcessTestCase.php
index 30ff362..9d39494 100644
--- a/core/modules/migrate/tests/src/Unit/process/MigrateProcessTestCase.php
+++ b/core/modules/migrate/tests/src/Unit/process/MigrateProcessTestCase.php
@@ -8,7 +8,6 @@
 namespace Drupal\Tests\migrate\Unit\process;
 
 use Drupal\Tests\migrate\Unit\MigrateTestCase;
-use Drupal\migrate\MigrateSkipRowException;
 
 abstract class MigrateProcessTestCase extends MigrateTestCase {
 
diff --git a/core/modules/migrate/tests/src/Unit/process/MigrationTest.php b/core/modules/migrate/tests/src/Unit/process/MigrationTest.php
index 1474987..77fb8be 100644
--- a/core/modules/migrate/tests/src/Unit/process/MigrationTest.php
+++ b/core/modules/migrate/tests/src/Unit/process/MigrationTest.php
@@ -7,7 +7,6 @@
 namespace Drupal\Tests\migrate\Unit\process;
 
 use Drupal\migrate\Plugin\migrate\process\Migration;
-use Drupal\migrate\MigrateSkipRowException;
 
 /**
  * Test the Migration process plugin.

Removing unused uses on commit.

  • alexpott committed 04fdb83 on 8.0.x 
    Issue #2337749 by benjy: Fixed no_stub is never used.

Status: Fixed » Closed (fixed)

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