Closed (fixed)
Project:
Drupal core
Version:
9.4.x-dev
Component:
rdf.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
3 Mar 2022 at 09:57 UTC
Updated:
14 Apr 2022 at 09:39 UTC
Jump to comment: Most recent, Most recent file
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!