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

CommentFileSizeAuthor
#9 2293105-cleanup-migrate-config-schema-9.patch5.67 KBvijaycs85
#6 2293105-cleanup-migrate-config-schema-6.patch28.27 KBvijaycs85
#5 2293105-diff-2-5.txt23.71 KBvijaycs85
#5 2293105-cleanup-migrate-config-schema-5.patch28.44 KBvijaycs85
#2 2293105-cleanup-migrate-config-schema-2.patch5.67 KBvijaycs85

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

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

vijaycs85
Location London, UK
commented
Title: Clean up MigrateConfigSchemaBase by using SchemaCheckTestTrait » Clean up MigrateActionConfigSchemaTest by using SchemaCheckTestTrait
Status: Active » Needs review
StatusFileSize
new 2293105-cleanup-migrate-config-schema-2.patch5.67 KB

Here we go.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

That looks fantastic :) RTBC if/when passes.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community
vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2293105-cleanup-migrate-config-schema-5.patch28.44 KB
new 2293105-diff-2-5.txt23.71 KB

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

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Title: Clean up MigrateActionConfigSchemaTest by using SchemaCheckTestTrait » Add config schema test to all configuration test in migration
StatusFileSize
new 2293105-cleanup-migrate-config-schema-6.patch28.27 KB 
+++ 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

vijaycs85
Location London, UK
commented
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
StatusFileSize
new 2293105-cleanup-migrate-config-schema-9.patch5.67 KB

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

vijaycs85
Location London, UK
commented
Status: Reviewed & tested by the community » Needs review
vijaycs85’s picture

vijaycs85
Location London, UK
commented
Status: Needs review » Reviewed & tested by the community

As per #4

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
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

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
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.