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.
Comment | File | Size | Author |
---|---|---|---|
#33 | 2346039-32.patch | 15.18 KB | benjy |
#31 | interdiff.txt | 1.17 KB | benjy |
#31 | 2346039-31.patch | 15.18 KB | benjy |
#23 | 2346039-23.patch | 16.11 KB | ultimike |
#16 | 2346039-16.patch | 14.54 KB | benjy |
Comments
Comment #1
ultimikeAttaching the patch to the MigrateDrupal6Test issue that add/reorganizes the list of migrations and dump files.
-mike
Comment #2
benjy CreditAttribution: benjy commentedNR for the bot
Comment #6
ultimikeUpdated 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:
-mike
Comment #7
benjy CreditAttribution: benjy commentedThis 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.
One of these bundles doesn't exist in the dump data and fixing will fix this issue.
Comment #8
ultimikeBenjy,
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
Comment #9
ultimikeI 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
Comment #10
ultimikeSince I have it handy, I'm attaching the results of my latest MigrateDrupal6Test.
-mike
Comment #11
ultimikeStill 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
Comment #12
ultimikeComment #13
ultimikeUgh - 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
Comment #14
ultimikeI 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:
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
Comment #15
svendecabooterIf 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.
Comment #16
benjy CreditAttribution: benjy commentedGood 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:
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
Comment #18
benjy CreditAttribution: benjy commentedCorrected the update in a dump file.
Comment #20
benjy CreditAttribution: benjy commentedOK, 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.
Comment #21
ultimikeBenjy,
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
Comment #22
ultimikePostponed until #2363643: Nodes with format 0 are skipped is resolved.
-mike
Comment #23
ultimikeNow 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
Comment #24
chx CreditAttribution: chx commentedThanks for the work this looks great!
Comment #27
alexpottNew some new lines at the end of these files.
Moving books implementation here is wrong - this should be moving it's own implementation.
Should be @see
Comment #28
alexpottAnd 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.
Comment #29
benjy CreditAttribution: benjy commented@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.
Comment #30
pushkarpathak CreditAttribution: pushkarpathak commentedOn 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
Comment #31
benjy CreditAttribution: benjy commentedNow #2363647: Cannot programatically update books has been fixed, i've re-rolled this patch against head. Interdiff shows the work around removed.
Comment #32
chx CreditAttribution: chx commentedExciting!
Comment #33
benjy CreditAttribution: benjy commentedUploading the same patch again because I think I was a little quick and uploaded that last patch before the last issue was pushed.
Comment #35
alexpottThis 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!
Fixed on commit