Problem/Motivation
Make sure the functional tests in migrate_drupal_ui work without RDF installed and that the migration tests in RDF still work when RDF is removed from core.
Steps to reproduce
Proposed resolution
- Add @todo where needed to remove RDF references in the removal issue
- Make a new smaller d7 fixture for RDF and update cspell.json to ignore it
- Create a ReviewPageTest to confirm that RDF will be upgraded when enabled on the destination
- Copy and modify Upgrade7Test
- \Drupal\Tests\migrate_drupal\Unit\MigrationStateUnitTest. This is a unit test and uses 'rdf' as a module name only it does not need to be changed.
Remaining tasks
Review
Commit
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 3267513-15.patch | 762.26 KB | andregp |
| #15 | interdiff_3267513_11-15.txt | 1.15 KB | andregp |
| #11 | 3267513-11.patch | 762.39 KB | quietone |
| #11 | interdiff-9-11.txt | 2.48 KB | quietone |
| #9 | 3267513-9.patch | 762.95 KB | quietone |
Comments
Comment #2
quietone commentedMaking a start and testing on 9.4.x since I was working there and not on 10.
Todo: \Drupal\Tests\migrate_drupal\Unit\MigrationStateUnitTest
Comment #3
quietone commentedAlso, fix the @todo, some are copy/paste from tracker
Comment #4
quietone commentedI made a new, smaller test fixture for RDF and greatly simplified the tests in the RDF module. The changes are significant and I don't think an interdiff will help, especially as the previous patch has the fixture in it. I am uploading a review patch that does not contain the fixture to make it easier to review.
The links in the @todo are fixed.
Still todo: \Drupal\Tests\migrate_drupal\Unit\MigrationStateUnitTest
Comment #6
quietone commentedThe failing test is unrelated, Drupal\FunctionalJavascriptTests\Ajax\AjaxTest.
Comment #7
quietone commentedRerolling
Comment #8
quietone commenteds/tracker/RDF in the IS.
Comment #9
quietone commentedAdd more @todo and fix errors comments. No code changes so not running the tests.
Comment #10
danflanagan8@quietone,
after applying the (real) patch in #9 I'm still seeing a number of references to rdf inNow I see that's a remaining task!Drupal\Tests\migrate_drupal\Unit\MigrationStateUnitTest. I can't claim I fully understand what that test does, but I suspect we need to do something with those references to rdf.Also, in the (new)
Drupal\Tests\rdf\Functional\Migrate::Upgrade7TestI *think* this needs rdf added to the$modulesarray since rdf will no longer get enabled automatically once it is removed from the standard profile. Or maybe that happens when it is actually removed from the standard profile?That's all that jumped out at me. Looking good! I guess I'll throw this back to NW.
Comment #11
quietone commentedUpdated IS about MigrationStateUnitTest.
Yes, 'rdf' does need to be installed and it is a but tricky to know which issue to do it in. But I do think here is right, and besides Upgrad7Test needs some simplification anyway.
Comment #13
quietone commentedRandom fail, 1) Drupal\Tests\quickedit\FunctionalJavascript\QuickEditFileTest::testRemove
Comment #14
danflanagan8Thanks, @quietone!
My concerns in #10 are completely addressed. But regarding
getMissingPathsthing that I brought up in the migrate meeting: We intend to leave RDF ingetMissingPathsin the D7 migrate_drupal_ui ReviewPage tests, right?If I haven't somehow misunderstood, I think we need to remove the @todos in the
getMissingPathmethods.Comment #15
andregp commentedCorrected patch as per #14.
Comment #16
danflanagan8This patch is correct per my understanding of the "getMissingPaths" plan. I'm going to RTBC it.
Comment #19
catchThis looks fine to me - various @todos but all because it's a pre-requisite to the removal issue.
Committed/pushed to 10.0.x and cherry-picked to 9.4.x thanks!