Just that, remove /core/modules/migrate/tests/src/Unit/MigrateSqlSourceTestCase

CommentFileSizeAuthor
#16 309699-16.patch18.41 KBquietone
#16 interdiff-14-16.txt892 bytesquietone
#14 309699-14.patch18 KBquietone
#14 interdiff-7-14.txt628 bytesquietone
#7 interdiff-5-7.txt2.66 KBquietone
#7 3096991-7.patch17.39 KBquietone
#5 interdiff-2-5.patch2.52 KBquietone
#5 3096991-5.patch14.69 KBquietone
#2 3096991-2.patch12.2 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Active » Needs review
FileSize
3096991-2.patch12.2 KB

This removes the test base class and tests using it. Still need to confirm that test coverage isn't being lost.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented

Four legacy tests were removed and here are the existing tests that do the same work.

migrate_drupal/tests/src/Unit/source/VariableMultiRowTestBase.php replaced by migrate_drupal/tests/src/Kernel/Plugin/migrate/source/VariableMultiRowTest.php
migrate_drupal/tests/src/Unit/source/VariableTest.php replaced by core/modules/migrate_drupal/tests/src/Kernel/Plugin/migrate/source/VariableTest.php
migrate_drupal/tests/src/Unit/source/d6/VariableTranslationTest.php replaced by core/modules/migrate_drupal/tests/src/Kernel/Plugin/migrate/source/d6/VariableTranslationTest.php
migrate_drupal/tests/src/Unit/source/d6/i18nVariableTest.php replaced by core/modules/migrate_drupal/tests/src/Kernel/Plugin/migrate/source/d6/VariableTranslationTest.php

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Patch looks good and the existing tests above look correct too.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Reviewed & tested by the community » Needs review
FileSize
3096991-5.patch14.69 KB
interdiff-2-5.patch2.52 KB

@longwave, thanks but on a second look I think this should also remove the deprecated source plugins that were being tested.

The last submitted patch, 5: 3096991-5.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
FileSize
3096991-7.patch17.39 KB
interdiff-5-7.txt2.66 KB

Seem there were two test of the deprecated i18nVariable source plugin. One in namespace Drupal\Tests\migrate_drupal\Kernel\Plugin\migrate\source\d6 and one in Drupal\Tests\migrate_drupal\Unit\source\d6.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Title: Remove deprecated MigrateSqlSourceTestBase » Remove deprecated MigrateSqlSourceTestCase
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Deprecation Removal
Parent issue: » #2716163: [META] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 9 branch

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 3096991-7.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Reviewed & tested by the community

Random fail in Drupal\Tests\media_library\FunctionalJavascript\WidgetViewsTest

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Queued a test run on PHP 7.3

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work

There's an @see \Drupal\migrate_drupal\Plugin\migrate\source\d6\i18nVariable in \Drupal\migrate\Plugin\migrate\destination\Config that needs replacing. I think to \Drupal\migrate_drupal\Plugin\migrate\source\d6\VariableTranslation but not 100% sure.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-7-14.txt628 bytes
309699-14.patch18 KB

Thanks alexpott.

The @see is changed to \Drupal\migrate_drupal\Plugin\migrate\source\d6\VariableTranslation.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Needs work

Verified that the change in #14 is correct.

+++ b/core/modules/migrate/src/Plugin/migrate/destination/Config.php
@@ -63,7 +63,7 @@
  * coupled with the relevant langcode as obtained from the "i18n_variable"
  * source plugin.

… but this comment also needs to be updated.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-14-16.txt892 bytes
309699-16.patch18.41 KB

Updated the doc bloc in /core/modules/migrate/src/Plugin/migrate/destination/Config.php. grepped for i18n_variable and didn't see others that needed to be changed. I could be wrong!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy at Acquia commented
Status: Needs review » Reviewed & tested by the community

Looks good now :)

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed eb9af30 and pushed to 9.0.x. Thanks!

Added issue credit @Wim Leers and myself as we left reviews that improved the patch.

Fixing long comment on commit...

diff --git a/core/modules/migrate/src/Plugin/migrate/destination/Config.php b/core/modules/migrate/src/Plugin/migrate/destination/Config.php
index 9f65b3ade8..13f7e908e0 100644
--- a/core/modules/migrate/src/Plugin/migrate/destination/Config.php
+++ b/core/modules/migrate/src/Plugin/migrate/destination/Config.php
@@ -60,8 +60,8 @@
  *
  * This will add the value of the variable "site_offline_message" to the config
  * with the machine name "system.maintenance" as "system.maintenance.message",
- * coupled with the relevant langcode as obtained from the "d6_variable_translation"
- * source plugin.
+ * coupled with the relevant langcode as obtained from the
+ * "d6_variable_translation" source plugin.
  *
  * @see \Drupal\migrate_drupal\Plugin\migrate\source\d6\VariableTranslation
  *

  • alexpott committed eb9af30 on 9.0.x 
    Issue #3096991 by quietone, alexpott, Wim Leers: Remove deprecated...

Status: Fixed » Closed (fixed)

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