Problem/Motivation

Make sure the functional tests in migrate_drupal_ui work without color installed and that the migration tests in color still work when tracker is removed from core.

Steps to reproduce

Proposed resolution

  • Add @todo where needed to remove Color references in the removal issue
  • Make a new smaller d7 fixture for Color and update cspell.json to ignore it
  • Create a ReviewPageTest to confirm that Color will be upgraded when enabled on the destination
  • Copy and modify Upgrade7Test. This is not needed the migration is config only and thus is not in the entity counts that are tested in the Upgrade tests.

\Drupal\Tests\migrate_drupal\Unit\MigrationStateUnitTest uses the 'color' in source data, it is not relying on the color module at all.

Remaining tasks

Patch
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#11 reroll_diff_3270897_6_3270897_11.txt1.22 KByogeshmpawar
#11 3270897-11.patch760.01 KByogeshmpawar
#7 3270897-6-review-only.patch6.99 KBquietone
#6 3270897-6.patch760.03 KBquietone
#6 interdiff-2-6.txt1.61 KBquietone
#3 3270897-2.patch760.83 KBquietone

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone commented
Status: Needs review » Active
quietone’s picture

quietone commented
Version: 10.0.x-dev » 9.4.x-dev
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 3270897-2.patch760.83 KB

The color migration is config only so this should be straightforward.

quietone’s picture

quietone commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 3: 3270897-2.patch, failed testing. View results

quietone’s picture

quietone commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2-6.txt1.61 KB
new 3270897-6.patch760.03 KB

Copy/paste errors.

quietone’s picture

quietone commented
StatusFileSize
new 3270897-6-review-only.patch6.99 KB

Adding a review only patch

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs review » Reviewed & tested by the community

The todo's mean the related issue #3270899: Remove Color module from core will probably need a reroll after commit, but I think that is fine.

The extra test works as expected. I've skimmed through the other patch but that is kinda hard with that fixture :)

Anyways, settings RTBC, thanks @quietone!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a re-roll (at least for 10.x)

bbrala’s picture

bbrala
Location Netherlands
commented

Ah, fair enough. I'll keep an eye out for the rerol

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 3270897-11.patch760.01 KB
new reroll_diff_3270897_6_3270897_11.txt1.22 KB

Rerolled the patch against 10.0.x branch & added reroll diff.

bbrala’s picture

bbrala
Location Netherlands
commented
Status: Needs review » Reviewed & tested by the community

New patch applies, and the changes that have been added are all trackable to recent commits. Looking good. RTBC again.

  • catch committed 8c0c151 on 10.0.x 
    Issue #3270897 by quietone, yogeshmpawar, bbrala: Handle migration tests...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!

  • catch committed 13a660b on 9.4.x 
    Issue #3270897 by quietone, yogeshmpawar, bbrala: Handle migration tests...

Status: Fixed » Closed (fixed)

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