Add config schema test to all configuration test in migration, fix bugs

Wow, good job! #2293063: Config schema mapping parsing is inconsistently forgiving, does not conform to the interface also found several issues with migration tests, but none of them are overlapping, so looks like we are fine doing these separately :)

Can we also get rid of MigrateActionConfigSchemaTest in favor of just a method in an existing test like the rest here? Now that #2293105: MigrateActionConfigSchemaTest duplicates all its code from SchemaCheckTestTrait landed :)

Since neither of them are used in D8 anymore, from what I can tell, lets remove them and we can figure out what to do in the relevant issues.

I agree its fine to remove them if those are not used in D8 anymore especially if you have issues to figure them out.

Working on #2292821: Use widget for comment subject field I found that migrate schema for comment migrations is inconsistent

+++ b/core/modules/migrate/config/schema/migrate.source.schema.yml
@@ -0,0 +1,388 @@
+migrate.source.d6_comment:
...
+migrate.source.d6_comment:

how does duplicated keys works?

I think this looks great for the scope of making migrates tested and indeed found several bugs in migrations :) Good stuff.

Really nice work!

+++ b/core/modules/migrate_drupal/src/Tests/d6/MigrateActionConfigsTest.php
@@ -55,4 +58,12 @@ public function testActionSettings() {
   }
 
+  /**
+   * Tests configuration schema.
+   */
+  public function testConfigSchema() {
+    $config_data = \Drupal::config('action.settings')->get();
+    $this->assertConfigSchema(\Drupal::service('config.typed'), 'action.settings', $config_data);
+  }
+
 }

I don't think we should be adding a separate test method to all the migrate tests. This should be combined with testActionSettings since that Tests migration of action variables to action.settings.yml. and so does this. This will save 2 additional test set ups since these tests are run twice.

This pattern is repeated a lot throughout the patch - all instances need fixing.

Looks even better.

Committed acb96c0 and pushed to 8.x. Thanks!

Yay, thanks. So looks like the known schema problems are in #2295737: Not all shipped configuration passes validation even with all modules enabled and #2293773: Field allowed values use dots in key names - not allowed in config.

The migration system uses the config API. Why doesn't the config API enforce the schema? Are we going to need to test every module that uses the config API?? This shouldn't be necessary and should be rolled back.

Config save checks the schema now. It did not check it yet at the time of this issue because our schemas were far from complete. I proposed removing these explicit tests at the time, but @alexpott said they do no harm.

As @Gábor Hojtsy pointed out we do have automated testing of config schema now and it now test migrate's use of config too so yes it can be removed I stand by the tests doing no harm. @chx is you want to remove the tests - fair enough - but new issue please.