Problem/Motivation

I was updating the complete D6->D8 manifest for testing (see https://www.drupal.org/node/2221779), I ran a very simple full migration on my local and was stopped by the following error:

...
Running d6_menu_settings                        [ok]
Running d6_node_setting_promote            [ok]
Drush command terminated abnormally due to an unrecoverable error.            [error]
Error: Call to a member function getConfigDependencyName() on a non-object in /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Field/FieldConfigBase.php, line 236

I was curious about this, so I ran the MigrateNodeBundleSettingsTest on my local and it passed fine. I then ran the MigrateDrupal6Test and it passed fine as well. I then checked the list of migrations in the MigrateDrupal6Test and noticed that the d6_node_setting_promote (and other newer migrations) were missing.

I then went through the entire MigrateDrupal6Test and compared both the migrations and dump files to the current codebase. There were a number of missing migrations and dump files, so I decided to add the missing ones and reorganize everything so that the lists match the list of current migrations and dump files.

Missing migrations:

  • d6_book
  • d6_node_setting_promote
  • d6_node_setting_status
  • d6_node_setting_sticky
  • d6_system_logging
  • d6_user_contact_settings

Missing dumps:

  • Drupal6Book.php
  • Drupal6SystemLogging.php
  • Drupal6UploadField.php

Missing tests:

  • MigrateBookTest
  • MigrateNodeBundleSettingsTest
  • MigrateSystemLoggingTest
  • MigrateUserContactSettingsTest

Once this was complete, I re-ran the MigrateDrupal6Test and received the exact same error as above.

Proposed resolution

Fix the issue reported above and hope that there are no other issues that result from the missing migrations and dump files.

Remaining tasks

Fix the d6_node_setting_promote issue above.

User interface changes

Zip.

API changes

Zilch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ultimike’s picture

FileSize
5.29 KB

Attaching the patch to the MigrateDrupal6Test issue that add/reorganizes the list of migrations and dump files.

-mike

benjy’s picture

Status: Active » Needs review

NR for the bot

Status: Needs review » Needs work

The last submitted patch, 1: 2346039-1-FAIL.patch, failed testing.

Status: Needs work » Needs review

Pedro Lozano queued 1: 2346039-1-FAIL.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2346039-1-FAIL.patch, failed testing.

ultimike’s picture

FileSize
9.08 KB

Updated patch attached, added/reordered list of tests to run.

No need to submit this to the test bot, it will still fail...

Here's the current fatal error output:

Fatal error: Call to a member function getConfigDependencyName() on a non-object in /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Field/FieldConfigBase.php on line 236

Call Stack:
    0.0099     420888   1. {main}() /Users/michael/Sites/drupal8/core/scripts/run-tests.sh:0
    3.3359   15023760   2. simpletest_script_run_one_test(???, ???) /Users/michael/Sites/drupal8/core/scripts/run-tests.sh:39
    3.3489   16533224   3. Drupal\simpletest\TestBase->run(???) /Users/michael/Sites/drupal8/core/scripts/run-tests.sh:631
  226.6323   77725488   4. Drupal\migrate_drupal\Tests\MigrateFullDrupalTestBase->testDrupal() /Users/michael/Sites/drupal8/core/modules/simpletest/src/TestBase.php:860
  228.9868   76633800   5. Drupal\migrate\MigrateExecutable->import() /Users/michael/Sites/drupal8/core/modules/migrate_drupal/src/Tests/MigrateFullDrupalTestBase.php:60
  229.0211   76703328   6. Drupal\migrate\Plugin\migrate\destination\EntityConfigBase->import(???, ???) /Users/michael/Sites/drupal8/core/modules/migrate/src/MigrateExecutable.php:285
  229.0414   76782200   7. Drupal\Core\Entity\Entity->save() /Users/michael/Sites/drupal8/core/modules/migrate/src/Plugin/migrate/destination/EntityConfigBase.php:45
  229.0416   76782200   8. Drupal\Core\Config\Entity\ConfigEntityStorage->save(???) /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Entity/Entity.php:325
  229.0417   76782288   9. Drupal\Core\Entity\EntityStorageBase->save(???) /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php:232
  229.0510   76782880  10. Drupal\Core\Field\Entity\BaseFieldOverride->preSave(???) /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Entity/EntityStorageBase.php:395
  229.0513   76783024  11. Drupal\Core\Config\Entity\ConfigEntityBase->preSave(???) /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php:162
  229.0603   76812400  12. Drupal\Core\Field\FieldConfigBase->calculateDependencies() /Users/michael/Sites/drupal8/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php:309

-mike

benjy’s picture

Issue tags: +Novice

This will be fixed by #2348875: Improving our dump files but it's a much bigger issue and I don't know how long it will take so i'm not against someone fixing this here.

The error you're getting is when a bundle is missing, so that puts us into MigrateNodeBundleSettingsTest.php because that runs the d6_node_setting_promote migration. The only difference between Drupal6Test and running them individually is the setUp method. So we see the following code.

    // Setup the bundles.
    entity_create('node_type', array('type' => 'test_page'))->save();
    entity_create('node_type', array('type' => 'test_planet'))->save();
    entity_create('node_type', array('type' => 'test_story'))->save();
    entity_create('node_type', array('type' => 'test_event'))->save();
    entity_create('node_type', array('type' => 'story'))->save();

One of these bundles doesn't exist in the dump data and fixing will fix this issue.

ultimike’s picture

Benjy,

Yeah, fixing #2348875 will definitely help with issues like this, but since we're currently missing tests from Drupal6Test I figured we should probably get this one fixed in the interim.

I was thinking the same thing about the bundles - but looking in the Drupal6NodeType dump, all five of those bundles are present (beginning around line 37)...

-mike

ultimike’s picture

Issue tags: -Novice
FileSize
11.86 KB

I banged on this one for another hour or so this morning, still no luck.

I tried adding a d6_node_type migration dependency to the d6_node_setting_promote, d6_node_setting_status, and d6_node_setting_sticky migrations (including adding the id_mappings to the MigrateNodeBundleSettingsTest), and while that test passes, the MigrateDrupal6Test fails:

Drupal\migrate_drupal\Tests\d6\MigrateDrupal6Test 726 passes 27 fails 36 exceptions

I'm not sure if this is an improvement over the error I was getting previously (see comment 6 above), but at least it is different(?)

Attaching a still-failling patch in case anyone wants to pick this one up...

Thanks,
-mike

ultimike’s picture

FileSize
163.42 KB

Since I have it handy, I'm attaching the results of my latest MigrateDrupal6Test.

-mike

ultimike’s picture

FileSize
17.36 KB
163.5 KB

Still failing, but making progress.

I'm guessing there's something wrong with the Drupal6Book dump file, as I'm getting an integrity constraint violation on the "book" table (see attached Drupal6Test.txt file).

Chatting with chx about it on IRC, he cleared a few things up for me asked me to add a comment to the d6_book.yml, which I did (included in this patch).

-mike

ultimike’s picture

Issue summary: View changes
ultimike’s picture

Ugh - this issue is becoming my great white whale...

I started fresh yesterday afternoon and believe I have the issue narrowed down to the book migration. The actual migration is fine, but in the MigrateBookTest setup(), 5 D8 nodes are created. When I attempted to add 5 new nodes (and corresponding revisions) to the Drupal6Node dump, that's when I start hitting errors running the MigrateDrupal6Test. I must be doing something incorrect in the way I'm adding additional nodes to the dump file.

So, my current task is to figure out why I can't seem to add even 1 new node to the Drupal6Node dump file and have the MigrateDrupal6Test pass (with or without the book migration even running!)

-mike

ultimike’s picture

FileSize
2.36 KB
3.2 KB
8.81 KB
6.01 KB
6.26 KB
149.01 KB

I think I hit a wall on this one - either that, or I need a fresh set of eyes on the problem. Here's a summary of where I'm at:

  1. In the issue summary, there are a number of missing migrations, tests, and dumps in Drupal6Test. This goal of this case is to get all of these missing items into Drupal6Test.
  2. I first tried added all the missing items and reordering everything, but I ended up spending way too much time trying to fix all the various test fails and exceptions - it was like playing whack-a-mole.
  3. Next, I tried a stepped approach. I first just reordered the dump files in Drupal6Test, without adding the missing dumps. The test passed.
  4. Next, I added the missing dumps to the Drupal6Test, and the test passed (2346039-14a.patch).
  5. Next, I started adding migrations. I was able to add the d6_system_logging and d6_user_contact_settings migrations and still have the Drupal6Test pass, but when I added the d6_node_setting_promote, d6_node_setting_status, and d6_node_setting_sticky migrations, things went south (2346039-14b.patch and the Drupal6Test results at 2346039-14b.results.txt).
  6. I was able to fix the issues created in the previous step by adding a dependency to d6_node_type on d6_node_setting_promote, d6_node_setting_status, and d6_node_setting_sticky migrations (2346039-14c.patch).
  7. Next, I added the d6_book migration - this is where things went bad for me. Lots of fails and exceptions (2346039-14d.patch and 2346039-14d.results.txt).

From what I've found, I _think_ the main issue is with the $this->assertIdentical($node->body->format, NULL); assertion in the MigrateNodeTest. It looks like that when a D6 node has a body format = 0, the node is stubbed. Prior to adding additional dumps, this isn't an issue and everything passes.

Then, when additional nodes are added to the Drupal7Node dump to support the MigrateBookTest, the stubbing of the body format = 0 node (source nid = 3) causes it to get a different (destination) nid, and then the $this->assertIdentical($node->body->format, NULL); assertion fails (because it is looking at destination node 3). I tried working around this issue by modifying MigrateNodeTest and Drupal7Node dump to have the test node for the body format = 0 assertion come last, and while that did help a bit, there were still issues.

I could use some advice on how best to proceed.

Thanks,
-mike

svendecabooter’s picture

If I understand the comments above correctly, this would be an issue with the tests, and the settings that are being generated to run that test?

Because I'm also getting this problem when I try to Drush migrate a content type with an image field attached to it - see https://www.drupal.org/node/2346415#comment-9278085.

Not sure if that's related to the reported issue here, or if I'm doing something wrong on my local install...
Just thought I'd give a heads up.

benjy’s picture

Status: Needs work » Needs review
FileSize
14.54 KB
8.84 KB

Good work so far Mike, Drupal6Test is quite the beast to debug but it sure knows how to turn out a few bugs!

I spent a few hours on this today and fixed the remaining issues, mainly:

  1. The nodes didn't exist in the dumps so the errors you were seeing were because the node migration tried to create the nodes rather than update existing ones.
  2. Next up was a bug in the book module, see #2363647: Cannot programatically update books
  3. Finally, I had to make sure the titles for the nodes in MigrateBookTest matched up with the nodes in Drupal6Node so the tree tests in MigrateBookTest passed.

I started from 14c by accident, I'm presuming d only enabled the book stuff which I did anyway?

I also found another issue, nodes with the format 0 gets translated to NULL and then we have no way of using NULL as a machine_name for the stub. #2363643: Nodes with format 0 are skipped

Status: Needs review » Needs work

The last submitted patch, 16: 2346039-16.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
FileSize
14.5 KB
545 bytes

Corrected the update in a dump file.

Status: Needs review » Needs work

The last submitted patch, 18: 2346039-18.patch, failed testing.

benjy’s picture

Status: Needs work » Needs review
FileSize
15.58 KB
1.63 KB

OK, I looked a little harder. I added a new node, node id 9 because node 3 is been used elsewhere in other tests. The test still fails, this time it doesn't get migrated which is because of #2363643: Nodes with format 0 are skipped. I've commented the test out for now and we can fix it in #2363643: Nodes with format 0 are skipped

We could make this work again but our dumps are so hard to work with, it will be easier to fix the issue than re-organise the dumps.

EDIT: I actually think the only reason the commented out test ever worked is because the test itself created a node with id 3 and the format defaulted to NULL.

ultimike’s picture

Benjy,

It would have taken me another week of head-banging to find the book bug and figure out the entity_type_alter() stuff. Great job.

I'm pretty sure we need someone else to review this, correct?

Thanks,
-mike

ultimike’s picture

Status: Needs review » Postponed

Postponed until #2363643: Nodes with format 0 are skipped is resolved.

-mike

ultimike’s picture

Status: Postponed » Needs review
FileSize
16.11 KB

Now that #2363643: Nodes with format 0 are skipped is fixed, I've re-rolled the patch with the format=0 test included. No interdiff - sorry!

I also found that MigrateNodeBundleSettingsTest, MigrateSystemLoggingTest, and MigrateUserContactSettingsTest were missing from the MigrateDrupal6Test, so I added them.

I still have the desire to re-order the lists of migrations, tests, and dumps in MigrateDrupal6Test so that they are alphabetical (so they match a typical file listing and are easier to check for omissions), but I'll wait until this issue is fixed and open a new issue just for the re-ordering. I did go through the lists manually, and I think we have everything at this point (which was a big part of the original intent of this issue).

-mike

chx’s picture

Title: MigrateDrupal6Test issue » Add missing migrations to MigrateDrupal6Test and fix the result
Status: Needs review » Reviewed & tested by the community

Thanks for the work this looks great!

The last submitted patch, 14: 2346039-14b.patch, failed testing.

The last submitted patch, 14: 2346039-14d.patch, failed testing.

alexpott’s picture

  1. +++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_promote.yml
    @@ -14,3 +14,6 @@ process:
    \ No newline at end of file
    
    +++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_status.yml
    @@ -14,3 +14,6 @@ process:
    \ No newline at end of file
    
    +++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_sticky.yml
    @@ -14,3 +14,6 @@ process:
    \ No newline at end of file
    

    New some new lines at the end of these files.

  2. +++ b/core/modules/migrate_drupal/migrate_drupal.module
    @@ -9,3 +11,28 @@ function migrate_drupal_entity_type_alter(array &$entity_types) {
    +      $group = $implementations['book'];
    +      unset($implementations['book']);
    +      $implementations['book'] = $group;
    

    Moving books implementation here is wrong - this should be moving it's own implementation.

  3. +    // See https://www.drupal.org/node/2363647
    

    Should be @see

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

And can the migrate_drupal_module_implements_alter be removed after https://www.drupal.org/node/2363647? I think so... so I'd be a fan of solving that issue first.

benjy’s picture

@alexpott, yes it could be removed after #2363647: Cannot programatically update books. My fear is that MigrateDrupal6Test is incredibly tedious to debug and the more that changes in migrate before this happens the more work we have to do here and it really is quite painful.

I've posted a possible fix #2363647: Cannot programatically update books but I'm not clued up on the book module, i'll also try track the book maintainer down for some insights.

pushkarpathak’s picture

On migration I am getting following error:
Fatal error: Call to a member function getConfigDependencyName() on a non-object in Core\Field\FieldConfigBase.php
on line 252

I am not using Book module and the migration was successful in beta2. I am getting above said error in beta3 migration.

Thanks
Pushkar

benjy’s picture

Status: Needs work » Needs review
FileSize
15.18 KB
1.17 KB

Now #2363647: Cannot programatically update books has been fixed, i've re-rolled this patch against head. Interdiff shows the work around removed.

chx’s picture

Status: Needs review » Reviewed & tested by the community

Exciting!

benjy’s picture

FileSize
15.18 KB

Uploading the same patch again because I think I was a little quick and uploaded that last patch before the last issue was pushed.

The last submitted patch, 31: 2346039-31.patch, failed testing.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed e789dec and pushed to 8.0.x. Thanks!

diff --git a/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_promote.yml b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_promote.yml
index eceb082..b2b3cc8 100644
--- a/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_promote.yml
+++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_promote.yml
@@ -16,4 +16,4 @@ destination:
   plugin: entity:base_field_override
 migration_dependencies:
   required:
-    - d6_node_type
\ No newline at end of file
+    - d6_node_type
diff --git a/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_status.yml b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_status.yml
index 6b603c7..adb2a7d 100644
--- a/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_status.yml
+++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_status.yml
@@ -16,4 +16,4 @@ destination:
   plugin: entity:base_field_override
 migration_dependencies:
   required:
-    - d6_node_type
\ No newline at end of file
+    - d6_node_type
diff --git a/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_sticky.yml b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_sticky.yml
index 5a2def5..988a0ac 100644
--- a/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_sticky.yml
+++ b/core/modules/migrate_drupal/config/install/migrate.migration.d6_node_setting_sticky.yml
@@ -16,4 +16,4 @@ destination:
   plugin: entity:base_field_override
 migration_dependencies:
   required:
-    - d6_node_type
\ No newline at end of file
+    - d6_node_type
diff --git a/core/modules/migrate_drupal/migrate_drupal.module b/core/modules/migrate_drupal/migrate_drupal.module
index fd9ada6..f76e67c 100644
--- a/core/modules/migrate_drupal/migrate_drupal.module
+++ b/core/modules/migrate_drupal/migrate_drupal.module
@@ -1,7 +1,5 @@
 <?php
 
-use Drupal\Core\Entity\EntityInterface;
-
 /**
  * Implements hook_entity_type_alter().
  */

Fixed on commit

  • alexpott committed e789dec on 8.0.x
    Issue #2346039 by ultimike, benjy: Add missing migrations to...

Status: Fixed » Closed (fixed)

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