Problem/Motivation
As per #2183983: Find hidden configuration schema issues, several block and block content module tests would fail if we turn on full schema checking. Here is a patch to expose those errors and fix them. A subset of issues there.
Proposed resolution
Find and fix the issues.
Remaining tasks
Fix them.
User interface changes
None.
API changes
None except fixed schemas.
Comment | File | Size | Author |
---|---|---|---|
#30 | 2379697-block-schema-fix-30-same-as-25.patch | 14.63 KB | Gábor Hojtsy |
#26 | interdiff.txt | 4.21 KB | Gábor Hojtsy |
#25 | interdiff.txt | 2.28 KB | Gábor Hojtsy |
#25 | 2379697-block-schema-fix-25.patch | 14.63 KB | Gábor Hojtsy |
#23 | 2379697-block-schema-fix-23.patch | 12.63 KB | Gábor Hojtsy |
Comments
Comment #1
Gábor HojtsyComment #3
Gábor HojtsyFixes for the fails:
1. The test view had a wrong field key, should be fields.
2. The block content translation test encounters content_translation.settings, which is impossible to provide schema for. See #2363155: content_translation.settings config is not scalable. To make these pass, we need to provide an ignore schema. I also removed the schema assert from ContentTranslationSettingsTest.php in favor of the strict checking test option.
3. The block tests use test_field types and link fields respectively and schemas were missing for those.
Comment #4
Gábor HojtsyIn #2183983-74: Find hidden configuration schema issues @Berdir removed the cached data setting, which is indeed not referenced anywhere. Also fixing the default value for the test field.
Comment #7
Gábor HojtsyPatch did not apply anymore. Rerolled. Also moved schema ignore fix to the checker. I should have remembered we don't have iteration support on ignore.
Comment #8
Gábor HojtsyComment #10
Gábor HojtsyResolving that PHP fatal by not invoking the method I am removing :)
Comment #12
Gábor HojtsyField settings schema keys were restructured in #2370305: Refactor field type configuration schemas for DX, easier to find errors, changed for those. Also found link_type field setting on links that I previously missed for some reason.
Comment #13
Gábor HojtsyComment #15
Gábor HojtsyRerolled due to #2316909: Revisit all built-in test/default views configuration in core.
Comment #17
Gábor HojtsyMarked #2358263: Adding missing configuration schema for views blocks a duplicate because the same fails can be reproduced here and they are covered by making DisplayBlockTest strict on config schema.
Incorporated the fixes from there. Also trying to remove the fields in the test blocks because there are no link_to_node title fields in views_test_data (no title field even!).
Comment #19
Gábor HojtsyThis is starting to get a bit unwieldy as the test views that are imported from views_test also expose their problems here... Anyway, we can get going with fixing those fails so long as there is an end in sight :D Fixing fails from testbot related to missing plugin_id's (no typing for schema on test fields).
Comment #21
Gábor HojtsySo the fails here are due to the views wizard. Duh. Opened #2381973: View wizard creates 'invalid' views out of the box, missing plugin_ids, insecure permissions. Postponing on that one.
Comment #22
Gábor Hojtsy#2381973: View wizard creates 'invalid' views out of the box, missing plugin_ids, insecure permissions landed. This needs a reroll.
Comment #23
Gábor HojtsyRerolled.
Comment #25
Gábor HojtsyWe should place blocks with label and not title, see docs and code of drupalPlaceBlock.
Comment #26
Gábor HojtsyNote sure anymore that the test views were related, let's test without them. Will incorporate the fixes to #2325269: Test and fix views in test_views directories against their configuration schema unless required here. Should make reviewing the changes easier.
Comment #28
Gábor HojtsyWas a bit carried away with aiming for simplicity :D We do need the test views fixes.
Comment #30
Gábor HojtsyReuploading #25 to avoid any confusion.
Comment #31
vijaycs85Looks good as the changes are just adding strict config schema check and few missing field plugin elements in test... This is good to go.
Comment #32
alexpottThis issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed f355ada and pushed to 8.0.x. Thanks!
Comment #34
Gábor HojtsyYay, thanks!