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

Comments

quietone created an issue. See original summary.

quietone’s picture

Status: Active » Needs review
StatusFileSize
new1.74 MB

Making a start and testing on 9.4.x since I was working there and not on 10.

Todo: \Drupal\Tests\migrate_drupal\Unit\MigrationStateUnitTest

quietone’s picture

Also, fix the @todo, some are copy/paste from tracker

quietone’s picture

StatusFileSize
new12.83 KB
new762.75 KB

I 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

Status: Needs review » Needs work

The last submitted patch, 4: 3267513-4.patch, failed testing. View results

quietone’s picture

Issue summary: View changes

The failing test is unrelated, Drupal\FunctionalJavascriptTests\Ajax\AjaxTest.

quietone’s picture

Status: Needs work » Needs review
StatusFileSize
new988 bytes
new762.76 KB

Rerolling

quietone’s picture

Issue summary: View changes

s/tracker/RDF in the IS.

quietone’s picture

StatusFileSize
new3.36 KB
new13.04 KB
new762.95 KB

Add more @todo and fix errors comments. No code changes so not running the tests.

danflanagan8’s picture

Status: Needs review » Needs work

@quietone, after applying the (real) patch in #9 I'm still seeing a number of references to rdf in 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. Now I see that's a remaining task!

Also, in the (new) Drupal\Tests\rdf\Functional\Migrate::Upgrade7Test I *think* this needs rdf added to the $modules array 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.

quietone’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new2.48 KB
new762.39 KB

Updated 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.

Status: Needs review » Needs work

The last submitted patch, 11: 3267513-11.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review

Random fail, 1) Drupal\Tests\quickedit\FunctionalJavascript\QuickEditFileTest::testRemove

danflanagan8’s picture

Status: Needs review » Needs work

Thanks, @quietone!

My concerns in #10 are completely addressed. But regarding getMissingPaths thing that I brought up in the migrate meeting: We intend to leave RDF in getMissingPaths in the D7 migrate_drupal_ui ReviewPage tests, right?

+++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/MultilingualReviewPageTest.php
@@ -41,6 +41,9 @@ class MultilingualReviewPageTest extends MultilingualReviewPageTestBase {
@@ -113,7 +116,6 @@ protected function getAvailablePaths() {

@@ -113,7 +116,6 @@ protected function getAvailablePaths() {
       'Phone',
       'Poll',
       'Profile',
-      'RDF',
       'Search',
       'Search embedded form',
       'Shortcut',
@@ -167,6 +169,8 @@ protected function getMissingPaths() {

@@ -167,6 +169,8 @@ protected function getMissingPaths() {
       'Options',
       'Path translation',
       'Picture',
+      // @todo Remove RDF in https://www.drupal.org/node/3267515
+      'RDF',
       'References',
       'References UUID',
       'Translation redirect',

If I haven't somehow misunderstood, I think we need to remove the @todos in the getMissingPath methods.

andregp’s picture

Status: Needs work » Needs review
StatusFileSize
new1.15 KB
new762.26 KB

Corrected patch as per #14.

danflanagan8’s picture

Status: Needs review » Reviewed & tested by the community

This patch is correct per my understanding of the "getMissingPaths" plan. I'm going to RTBC it.

  • catch committed 337a873 on 10.0.x
    Issue #3267513 by quietone, andregp, danflanagan8: Handle migration...

  • catch committed 351a2c4 on 9.4.x
    Issue #3267513 by quietone, andregp, danflanagan8: Handle migration...
catch’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Fixed

This 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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.