Problem/Motivation

Writing a contrib test, it's useful to use MigrateDrupal6TestBase. The problem is you can't substitute your own fixture, forcing you to use MigrateDrupalTestBase.

Proposed resolution

Allow subclasses to change the fixtures.

Remaining tasks

Patch.

User interface changes

None.

API changes

Additional protected property that can change.

Data model changes

None.

CommentFileSizeAuthor
#10 2748467-migrate-subclass-fixtures-10.patch1.43 KBsam152
#2 2748467-migrate-subclass-fixtures-2.patch1.53 KBsam152

Comments

Sam152 created an issue. See original summary.

sam152’s picture

sam152 commented
Status: Active » Needs review
StatusFileSize
new 2748467-migrate-subclass-fixtures-2.patch1.53 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2748467-migrate-subclass-fixtures-2.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

Sam152, do you really need to substitute the entire fixture, or just to make small modifications to it? For the latter, please take a look at MigrateDumpAlterInterface::migrateDumpAlter().

sam152’s picture

sam152 commented

Working with a DB connection object to create new fields seems very non-trivial. Why is should core be given the Drupal UI as a tool for creating fixtures and contrib authors need to cherry-pick the various and very specific DB insertions/alterations required to achieve the same results?

The last submitted patch, 2: 2748467-migrate-subclass-fixtures-2.patch, failed testing.

benjy’s picture

benjy commented

This seems like a simple improvement to make things easier for contrib. We do not want to recommend MigrateDumpAlterInterface, that was added for a special case on the file migration, if anything it would be nice to remove that interface in the future.

Looks like the patch fails because of a syntax error otherwise +1 from me for RTBC.

sam152’s picture

sam152 commented

Expressions aren't allowed as property defaults. Worth moving into it's own protected method? ::loadDrupal6Fixture ::loaDrupal7Fixture?

benjy’s picture

benjy commented

Yeah we lets add a method.

sam152’s picture

sam152 commented
Status: Needs work » Needs review
StatusFileSize
new 2748467-migrate-subclass-fixtures-10.patch1.43 KB
benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/migrate_drupal/tests/src/Kernel/d6/MigrateDrupal6TestBase.php
@@ -28,7 +28,14 @@
+   * Get the path to the fixture file.

+++ b/core/modules/migrate_drupal/tests/src/Kernel/d7/MigrateDrupal7TestBase.php
@@ -14,7 +14,14 @@
+   * Get the path to the fixture file.

These should be "Gets", prob fix on commit.

alexpott’s picture

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

Committed 32b60ac and pushed to 8.1.x and 8.2.x. Thanks!

Committed to 8.1.x as well because migrate is experimental and this is about testing.

diff --git a/core/modules/migrate_drupal/tests/src/Kernel/d6/MigrateDrupal6TestBase.php b/core/modules/migrate_drupal/tests/src/Kernel/d6/MigrateDrupal6TestBase.php
index 3c3962f..72a8095 100644
--- a/core/modules/migrate_drupal/tests/src/Kernel/d6/MigrateDrupal6TestBase.php
+++ b/core/modules/migrate_drupal/tests/src/Kernel/d6/MigrateDrupal6TestBase.php
@@ -32,7 +32,7 @@ protected function setUp() {
   }
 
   /**
-   * Get the path to the fixture file.
+   * Gets the path to the fixture file.
    */
   protected function getFixtureFilePath() {
     return __DIR__ . '/../../../fixtures/drupal6.php';
diff --git a/core/modules/migrate_drupal/tests/src/Kernel/d7/MigrateDrupal7TestBase.php b/core/modules/migrate_drupal/tests/src/Kernel/d7/MigrateDrupal7TestBase.php
index 8291f3b..3afa82d 100644
--- a/core/modules/migrate_drupal/tests/src/Kernel/d7/MigrateDrupal7TestBase.php
+++ b/core/modules/migrate_drupal/tests/src/Kernel/d7/MigrateDrupal7TestBase.php
@@ -18,7 +18,7 @@ protected function setUp() {
   }
 
   /**
-   * Get the path to the fixture file.
+   * Gets the path to the fixture file.
    */
   protected function getFixtureFilePath() {
     return __DIR__ . '/../../../fixtures/drupal7.php';

Fixed on commit.

  • alexpott committed da40012 on 8.2.x 
    Issue #2748467 by Sam152: Allow children of migration test bases to...

  • alexpott committed 32b60ac on 8.1.x 
    Issue #2748467 by Sam152: Allow children of migration test bases to...

Status: Fixed » Closed (fixed)

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