Problem/Motivation
Over in #2533886: [meta] Move module-specific migration support into the particular modules supported, we've decided to split the Drupal 6 and 7 migration paths into their constituent modules. It's pretty straightforward, but the work is breaking MigrateDrupal6Test (an integration test of a full Drupal 6 site migration) because this test relies on a hard-coded laundry list of test classes.
Proposed Resolution
MigrateFullDrupalTestBase (the parent class of MigrateDrupal6Test) should discover tests dynamically, using the test_discovery service. This will fix the problem and ensure that we can easily add more integration tests without having to alter MigrateDrupal6Base (or its eventual Drupal 7 counterpart).
Remaining Tasks
Review the patch and commit.
API Changes
None.
UI Changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff-2534158-9-12.txt | 3.14 KB | phenaproxima |
| #12 | 2534158-12.patch | 7.39 KB | phenaproxima |
| #9 | interdiff-2534158-1-9.txt | 2.53 KB | phenaproxima |
| #9 | 2534158-9.patch | 8.54 KB | phenaproxima |
| #1 | 2534158-1.patch | 6.86 KB | phenaproxima |
Comments
Comment #1
phenaproximaComment #2
mikeryanComment #3
mikeryanNice, I'll be happy to rtbc once testbot approves.
Comment #4
phenaproximaComment #6
mikeryanOne of those testbot race condition things?
Comment #9
phenaproximaFixed, but I'm sorry to say it involved an ugly, though wholly necessary (albeit temporary, thankfully) kludge. I left a detailed comment in MigrateFullDrupalTestBase -- see the interdiff. It's a tricky little issue, and I couldn't see any other way out of it.
I don't think we'll have any choice but to commit this to core with the hack for now. Once #2533886: [meta] Move module-specific migration support into the particular modules supported lands we can remove it in a follow-up issue.
Comment #10
mikeryanQQ (without looking carefully at the tests in question) - are all tests that need to be run via MigrateFullDrupalTestBase destined to move to other modules, or might there be some outliers that need to remain in migrate_drupal? And if there are outliers, would they prevent us from removing the hack?
Comment #11
phenaproximaIn a word: yes. I thought about that and what I think might be a good solution is that, if there are outliers, we should simply change their group to migrate_drupal_6 and migrate_drupal_7, as appropriate, and filter on that instead of testing the class name against a regex.
There was an issue open to change the existing tests to use those groups, but that patch was big and sweeping and almost entirely obviated by #2533886: [meta] Move module-specific migration support into the particular modules supported, so I closed it. I didn't want to let it block this issue. So I'm thinking we can commit this with the hack for now, and when the dust settles in the meta-issue, we can revisit this if any outliers remain.
Comment #12
phenaproximaAt @mikeryan's suggestion, created a blacklist array in MigrateFullDrupalTestBase, rather than an awkward namespace-checking hack. Seems to solve the problem...and it's a much better way to go about fixing the trouble I found in #9.
Comment #13
chx commentedWe discussed this and determined that the blacklist functionality does not need to be extensible since the one blacklisted is an API-ish test (as much as migrate drupal has an API) and contrib will actually tests that test migrations.
Comment #14
mikeryan+1 on the RTBC.
Comment #15
alexpottMigrate is not subject to beta evaluation. Committed 978fcf0 and pushed to 8.0.x. Thanks!