Follow-up to #2183957: Provide configuration schema for Migration module

Problem/Motivation

As mentioned in #2183957-59: Provide configuration schema for Migration module, we have added MigrateConfigSchemaBase by duplicating the same code in ConfigSchemaBase which has been by SchemaCheckTestTrait

Proposed resolution

Remove duplicate code and use SchemaCheckTestTrait.

Remaining tasks

User interface changes

N/A

API changes

N/A

Comments

Gábor Hojtsy’s picture

Yeah I noticed this while working on #2293063: Config schema mapping parsing is inconsistently forgiving, does not conform to the interface. Maybe postpone that on this one, so we can avoid needing to adjust that code? It would be great to get this in ASAP :)

vijaycs85’s picture

Title: Clean up MigrateConfigSchemaBase by using SchemaCheckTestTrait » Clean up MigrateActionConfigSchemaTest by using SchemaCheckTestTrait
Status: Active » Needs review
FileSize
5.67 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,725 pass(es). View

Here we go.

Gábor Hojtsy’s picture

That looks fantastic :) RTBC if/when passes.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community
vijaycs85’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
28.44 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,822 pass(es), 11 fail(s), and 0 exception(s). View
23.71 KB

Sorry for taking it back to review, but this would cover more config schemas from migration side.

vijaycs85’s picture

Title: Clean up MigrateActionConfigSchemaTest by using SchemaCheckTestTrait » Add config schema test to all configuration test in migration
FileSize
28.27 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,797 pass(es), 11 fail(s), and 1 exception(s). View
+++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateAggregatorConfigsTest.php
@@ -62,4 +65,12 @@ public function testAggregatorSettings() {
+    $config_data = $config = \Drupal::config('aggregator.settings')->get();

+++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateContactConfigsTest.php
@@ -66,4 +69,13 @@ public function testContactSettings() {
+    $config_data = $config = \Drupal::config('contact.settings')->get();

+++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateDblogConfigsTest.php
@@ -55,4 +58,13 @@ public function testBookSettings() {
+    $config_data = $config = \Drupal::config('dblog.settings')->get();

and so on...

Removing all of them...

Status: Needs review » Needs work

The last submitted patch, 6: 2293105-cleanup-migrate-config-schema-6.patch, failed testing.

The last submitted patch, 5: 2293105-cleanup-migrate-config-schema-5.patch, failed testing.

vijaycs85’s picture

Title: Add config schema test to all configuration test in migration » Clean up MigrateActionConfigSchemaTest by using SchemaCheckTestTrait
Status: Needs work » Reviewed & tested by the community
FileSize
5.67 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 72,831 pass(es). View

Ok, this is good. All the 11 fails in #6 are valid errors. However each of them have it's own history of issue(s) and we may not get them all fixed anytime soon. So let's go with #2 and make these new test coverage and fixes as separate issue - #2293419: Add config schema test to all configuration test in migration, fix bugs

Just uploading the same patch in 2 here as I'm reviewer/committer-friendly ;)

vijaycs85’s picture

Status: Reviewed & tested by the community » Needs review
vijaycs85’s picture

Status: Needs review » Reviewed & tested by the community

As per #4

Gábor Hojtsy’s picture

Title: Clean up MigrateActionConfigSchemaTest by using SchemaCheckTestTrait » MigrateActionConfigSchemaTest duplicates all its code from SchemaCheckTestTrait
Issue tags: +Quick fix

More dramatic title and quick fix tag to help this land sooner :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Wow, great catch. :)

Committed and pushed to 8.x. Thanks!

  • webchick committed 29a8390 on 8.x
    Issue #2293105 by vijaycs85: MigrateActionConfigSchemaTest duplicates...

Status: Fixed » Closed (fixed)

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