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
Bug A (code):
#3051252: Upgrade path for Multiupload Filefield Widget and Multiupload Imagefield Widget has added multiupload_filefield_widget and multiupload_imagefield_widget moduels to the Drupal 7 migration test database fixture, but the addition made in the fixture file's system table defined that these modules are placed into the sites/all/modules/contrib directory.
This breaks the consistency with other modules (all of the other contributed modules are placed into sites/all/modules) , and also conflicts with the documentation: Generating database fixtures for migration tests.
Bug B (documentation):
The documentation above is very outdated in case of the Drupal 7 fixture: it requires more contributed modules (this was also missed in #3051252: Upgrade path for Multiupload Filefield Widget and Multiupload Imagefield Widget).
Steps to reproduce
Follow steps of Generating database fixtures for migration tests.
Proposed resolution
Bug A (code):
Fix the location of multiupload_filefield_widget and multiupload_imagefield_widget.
Bug B (documentation):
Update the documentation.
Remaining tasks
Patch✅Doc update✅
User interface changes
Nothing.
API changes
Nothing.
Data model changes
Nothing.
Release notes snippet
Nothing needed.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | core-fix_d7_migration_db_fixture-3213621.patch | 1.81 KB | huzooka |
| Screenshot 2021-05-11 at 15.14.58.png | 387.43 KB | huzooka | |
Comments
Comment #2
huzookaComment #3
huzookaComment #4
huzookaUpdated the documentation, so from now, https://www.drupal.org/node/2583227/revisions/12292117/view contains the new modules.
The patch also should be fine.
Comment #5
quietone CreditAttribution: quietone as a volunteer commentedThe D7 fixture needs more work than this one fix and I think we should make them all in one issue. I started in #3024459: Fix D7 test fixture which I postponed on an issue that was one of the last issues (if not the last at the time) that was changing the fixture but it doesn't need to be postponed.
Comment #6
huzooka@quitone, I agree that the fixture needs more work, but:
I don't see any cons why this shouldn't or cannot be fixed now.
Comment #7
quietone CreditAttribution: quietone as a volunteer commented@huzooka, fair enough. The changes in the related issue are somewhat different. If/when you notice other problems with the d7 fixture, can you note them in #3024459: Fix D7 test fixture?
Comment #8
larowlanIs there anyway we can write a test case that checks the location of the fixtures is correct?
Comment #9
Wim LeersDo we really need test coverage for this honest tiny mistake? @huzooka already updated the official docs over at https://www.drupal.org/node/2583227.
It is increasingly unlikely that we add more contrib modules, so it's not really worth adding explicit test coverage for this IMHO: the ROI is tiny.
The process for adding more contributed modules to the Drupal 7 DB fixture is already explicitly documented at https://www.drupal.org/node/2583227, where the whole fixture creation/updating process is thoroughly documented. It says:
That's all this oversight is fixing :) I think tests are overkill here, because then we'll be testing the test fixture 🙃 If we do that, then we probably want more tests of the fixture?
Gently pushing back here — if you really want tests for this, I'll add them.
P.S.: In the past, when I wanted to add
assert()statements to test infrastructure itself, it's always been shot down. So this would definitely be a very new pattern 🤓Comment #10
quietone CreditAttribution: quietone as a volunteer commentedI agree, there is absolutely no reason for a test. There are no more issues in the migration system for adding another contrib module to the test fixture.
Comment #11
huzookaI think that nothing blocks this to be committed.
Comment #12
larowlanThanks, that makes sense to me @quietone, @Wim Leers
Comment #15
webchickAwesome sauce! Committed and pushed to 9.3.x, cherry-picked to 9.2.x. Thanks!