Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The Migrate UI was introduced to with only basic tests added in #2647470: Write tests. We should test each migration to at least show that it is working. At the moment the tests are green even though some of the migration failed.
Proposed resolution
Add more tests and work why some of the things are not migrating.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#21 | 2683579-21.patch | 53.45 KB | alexpott |
#21 | 2683579-21.test-only.patch | 4.26 KB | alexpott |
#19 | 2683579-19.patch | 53.45 KB | alexpott |
#19 | 12-19-interdiff.txt | 884 bytes | alexpott |
#12 | 2683579-12.patch | 53.3 KB | alexpott |
Comments
Comment #2
alexpottThis will fail.
Comment #3
alexpottThis fixes the telephone field migration... I'm not sure that failed is the correct status if the telephone module is not installed on the destination... but that is not the point of this patch.
Comment #6
alexpottSo this patch should be green - and now we have proper testing of setting the source_base_path through the UI!
Comment #8
alexpottHmmm weird... that test passes locally for me...
Comment #9
alexpottAh my gitignore causing me an issue... missing the temp file...
Comment #10
benjy CreditAttribution: benjy at PreviousNext commentedAdded the fix to run these tests locally because it gets me every time, and added the missing temp file.
Comment #11
benjy CreditAttribution: benjy at PreviousNext commentedI only made one change to the test while debugging so I think I can still RTBC this, looks good.
Comment #12
alexpottRerolled on top of the plugins patch - until that is in 8.1.x this is only eligible for 8.2.x
The only thing that had to change was how we loaded the migrations to see if they were successful.
Basically we can use the MigrationCreationTrait to do this for us.
Comment #14
alexpott#12 got tested against 8.1.x and not 8.2.x... testing now against 8.2.x
Comment #15
benjy CreditAttribution: benjy at PreviousNext commentedLooks good to me.
Comment #16
alexpottComment #17
xjmComment #18
catchWhy array_values()? Could use a comment if needed.
Also can't see what the added images are used for - there's no obvious test changes that indicate whether they'd pass or fail due to the presence of the images or not, or I'm missing something. Test-only patch might be enough to answer that.
Comment #19
alexpottFor reasons I don't quite understand the id map is really tricky to work with :( - added a comment. But this is not the fault of this issue.
Comment #20
catchStill missing what difference the images make.
Comment #21
alexpottComment #24
catchCommitted/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!
Comment #25
xjm