Add theme.json schema tests #44252
Add theme.json schema tests #44252
Conversation
alecgeatches
requested review from
ajlende,
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
github-actions
bot
added
the
First-time Contributor
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @alecgeatches! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
| } ); | ||
|
|
||
| it( 'compiles in strict mode', async () => { | ||
| const result = ajv.compile( themeSchema ); |
There was a problem hiding this comment.
See point 3 from this discussion about why compile() is being used here instead of validate():
The validation code no longer uses an object to validate the schema. This ended up being trickier than expected. Your suggested code change using
validateran successfully, but did not report and strict errors ontheme.json's schema. For example, I tried adding invalid properties like"test": "abc"to the schema which should fail Ajv'sstrictSchema, but no errors were reported. The only way I could get the meta-schema to be validated was by validating an actual data structure or using.compile():
It's unclear from Ajv's documentation why.validate('.../draft-04#', $themeSchema)or.validate($themeSchema)weren't reporting errors. The best I could find was the documentation forcompile()which mentions the meta-schema:The schema passed to this method will be validated against meta-schema unless
validateSchemaoption is false. If schema is invalid, an error will be thrown. See options.It seems like the meta-schema is only validated during compilation, so the code has been changed to use
compile()instead.
There was a problem hiding this comment.
@alecgeatches I found a couple notes about this in the Ajv docs and issues.
ajv.validateSchema(schema: object): boolean
Validates schema. This method should be used to validate schemas rather than
validatedue to the inconsistency ofuriformat in JSON Schema standard.
validateSchemaonly checks “syntax” (it validated schema as JSON against metaschema), it may still be semantically incorrect.
Looks like I was wrong about using validate for validating against a meta schema in the first place. And it's intentional that strict mode only applies to compile, so that's why we should be using it instead of validateSchema to validate our schema. 🙃
annezazu
added
the
Global Styles
There was a problem hiding this comment.
Might be worth adding a note in the code about using compile instead of validate. If there isn't an issue on Ajv's GitHub repo, it could be good to create one and link it in the comment.
Otherwise this all looks great! Thanks for separating out the test stuff into its own PR. 👍
|
Congratulations on your first merged pull request, @alecgeatches! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts: https://profiles.wordpress.org/me/profile/edit/ And if you don't have a WordPress.org account, you can create one on this page: https://login.wordpress.org/register Kudos! |
|
@ajlende Glad you were able to track down a source for the |
What?
See original PR #43801. This PR does not include nesting-specific changes, just the
theme.jsontest additions and fixes to the current schema. Relevant portions of the original PR discussion have been copied here.These changes were made:
theme-schema.test.jsbased on existing block schema tests. The existing schema failed validation with Ajv's strict mode againstdraft-04, mostly due to some missingtypeidentifiers and a handful of other small changes.Why?
From Ajv's strict mode documentation:
Strict mode testing fixes a few missing/unknown identifiers in the current schema, and should help prevent accidental mistakes going forward.
How
Some minor changes were made to the original schema to make it validate correctly against
draft-04. To view the original errors, check out thetest/integration/theme-schema.test.jstest file from the fork repository branch againsttrunkand use this command to run validation checks:Running the test gives this validation result:
Click to expand
$ npx wp-scripts test-unit-js --config test/unit/jest.config.js --no-cache theme-schema.test.js FAIL test/integration/theme-schema.test.js theme.json schema ✕ compiles in strict mode (82 ms) ● theme.json schema › compiles in strict mode strict mode: unknown keyword: "color" 17 | 18 | it( 'compiles in strict mode', async () => { > 19 | const result = ajv.compile( themeSchema ); | ^ 20 | 21 | expect( result.errors ).toBe( null ); 22 | } ); at checkStrictMode (node_modules/ajv/lib/compile/util.ts:211:28) at checkUnknownRules (node_modules/ajv/lib/compile/util.ts:27:22) at alwaysValidSchema (node_modules/ajv/lib/compile/util.ts:17:3) at node_modules/ajv/lib/vocabularies/applicator/properties.ts:23:65 at Array.filter (<anonymous>) at Object.code (node_modules/ajv/lib/vocabularies/applicator/properties.ts:23:33) at keywordCode (node_modules/ajv/lib/compile/validate/index.ts:523:9) at node_modules/ajv/lib/compile/validate/index.ts:265:9 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at iterateKeywords (node_modules/ajv/lib/compile/validate/index.ts:262:7) at groupKeywords (node_modules/ajv/lib/compile/validate/index.ts:241:7) at node_modules/ajv/lib/compile/validate/index.ts:233:38 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at schemaKeywords (node_modules/ajv/lib/compile/validate/index.ts:232:7) at typeAndKeywords (node_modules/ajv/lib/compile/validate/index.ts:161:3) at subSchemaObjCode (node_modules/ajv/lib/compile/validate/index.ts:147:3) at subschemaCode (node_modules/ajv/lib/compile/validate/index.ts:124:7) at KeywordCxt.subschema (node_modules/ajv/lib/compile/validate/index.ts:491:5) at applyPropertySchema (node_modules/ajv/lib/vocabularies/applicator/properties.ts:45:11) at Object.code (node_modules/ajv/lib/vocabularies/applicator/properties.ts:32:9) at keywordCode (node_modules/ajv/lib/compile/validate/index.ts:523:9) at node_modules/ajv/lib/compile/validate/index.ts:265:9 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at iterateKeywords (node_modules/ajv/lib/compile/validate/index.ts:262:7) at groupKeywords (node_modules/ajv/lib/compile/validate/index.ts:241:7) at node_modules/ajv/lib/compile/validate/index.ts:233:38 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at schemaKeywords (node_modules/ajv/lib/compile/validate/index.ts:232:7) at typeAndKeywords (node_modules/ajv/lib/compile/validate/index.ts:161:3) at subSchemaObjCode (node_modules/ajv/lib/compile/validate/index.ts:147:3) at subschemaCode (node_modules/ajv/lib/compile/validate/index.ts:124:7) at KeywordCxt.subschema (node_modules/ajv/lib/compile/validate/index.ts:491:5) at inlineRefSchema (node_modules/ajv/lib/vocabularies/core/ref.ts:40:26) at Object.code (node_modules/ajv/lib/vocabularies/core/ref.ts:21:12) at keywordCode (node_modules/ajv/lib/compile/validate/index.ts:523:9) at node_modules/ajv/lib/compile/validate/index.ts:228:21 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at schemaKeywords (node_modules/ajv/lib/compile/validate/index.ts:228:9) at typeAndKeywords (node_modules/ajv/lib/compile/validate/index.ts:161:3) at subSchemaObjCode (node_modules/ajv/lib/compile/validate/index.ts:147:3) at subschemaCode (node_modules/ajv/lib/compile/validate/index.ts:124:7) at KeywordCxt.subschema (node_modules/ajv/lib/compile/validate/index.ts:491:5) at node_modules/ajv/lib/vocabularies/applicator/allOf.ts:15:26 at Array.forEach (<anonymous>) at Object.code (node_modules/ajv/lib/vocabularies/applicator/allOf.ts:13:12) at keywordCode (node_modules/ajv/lib/compile/validate/index.ts:523:9) at node_modules/ajv/lib/compile/validate/index.ts:265:9 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at iterateKeywords (node_modules/ajv/lib/compile/validate/index.ts:262:7) at groupKeywords (node_modules/ajv/lib/compile/validate/index.ts:248:7) at node_modules/ajv/lib/compile/validate/index.ts:233:38 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at schemaKeywords (node_modules/ajv/lib/compile/validate/index.ts:232:7) at typeAndKeywords (node_modules/ajv/lib/compile/validate/index.ts:161:3) at subSchemaObjCode (node_modules/ajv/lib/compile/validate/index.ts:147:3) at subschemaCode (node_modules/ajv/lib/compile/validate/index.ts:124:7) at KeywordCxt.subschema (node_modules/ajv/lib/compile/validate/index.ts:491:5) at applyPropertySchema (node_modules/ajv/lib/vocabularies/applicator/properties.ts:45:11) at Object.code (node_modules/ajv/lib/vocabularies/applicator/properties.ts:32:9) at keywordCode (node_modules/ajv/lib/compile/validate/index.ts:523:9) at node_modules/ajv/lib/compile/validate/index.ts:265:9 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at iterateKeywords (node_modules/ajv/lib/compile/validate/index.ts:262:7) at groupKeywords (node_modules/ajv/lib/compile/validate/index.ts:241:7) at node_modules/ajv/lib/compile/validate/index.ts:233:38 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.block (node_modules/ajv/lib/compile/codegen/index.ts:680:20) at schemaKeywords (node_modules/ajv/lib/compile/validate/index.ts:232:7) at typeAndKeywords (node_modules/ajv/lib/compile/validate/index.ts:161:3) at node_modules/ajv/lib/compile/validate/index.ts:100:5 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at node_modules/ajv/lib/compile/validate/index.ts:61:45 at CodeGen.code (node_modules/ajv/lib/compile/codegen/index.ts:525:33) at CodeGen.func (node_modules/ajv/lib/compile/codegen/index.ts:699:24) at validateFunction (node_modules/ajv/lib/compile/validate/index.ts:60:9) at topSchemaObjCode (node_modules/ajv/lib/compile/validate/index.ts:94:3) at validateFunctionCode (node_modules/ajv/lib/compile/validate/index.ts:42:7) at Ajv.compileSchema (node_modules/ajv/lib/compile/index.ts:163:25) at Ajv._compileSchemaEnv (node_modules/ajv/lib/core.ts:718:24) at Ajv.compile (node_modules/ajv/lib/core.ts:370:34) at Object.<anonymous> (test/integration/theme-schema.test.js:19:22) at runMicrotasks (<anonymous>) ● theme.json schema › compiles in strict mode expect(jest.fn()).not.toHaveWarned(expected) Expected mock function not to be called but it was called with: ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesAppearanceTools\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesBorder\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesColor\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesLayout\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesSpacing\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesTypography\" (strictTypes)"], ["strict mode: use allowUnionTypes to allow union type keyword at \"#/definitions/settingsPropertiesTypography/properties/typography/properties/fontSizes/items/properties/fluid\" (strictTypes)"], ["strict mode: type \"integer\" not allowed by context \"string\" at \"#/definitions/settingsPropertiesTypography/properties/typography/properties/fontFamilies/items/properties/fontFace/items/properties/fontWeight/oneOf/1\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"additionalProperties\" at \"#/properties/core~1archives\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesAppearanceTools\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/properties/core~1button/allOf/1\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesColor\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesLayout\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesSpacing\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/settingsPropertiesTypography\" (strictTypes)"], ["strict mode: use allowUnionTypes to allow union type keyword at \"#/definitions/settingsPropertiesTypography/properties/typography/properties/fontSizes/items/properties/fluid\" (strictTypes)"], ["strict mode: type \"integer\" not allowed by context \"string\" at \"#/definitions/settingsPropertiesTypography/properties/typography/properties/fontFamilies/items/properties/fontFace/items/properties/fontWeight/oneOf/1\" (strictTypes)"], ["strict mode: missing type \"object\" for keyword \"properties\" at \"#/definitions/stylesProperties\" (strictTypes)"] 30 | function assertExpectedCalls() { 31 | if ( spy.assertionsNumber === 0 && spy.mock.calls.length > 0 ) { > 32 | expect( console ).not[ matcherName ](); | ^ 33 | } 34 | } 35 | at Object.assertExpectedCalls (packages/jest-console/src/index.js:32:4) at runMicrotasks (<anonymous>) at TestScheduler.scheduleTests (node_modules/@jest/core/build/TestScheduler.js:333:13) at runJest (node_modules/@jest/core/build/runJest.js:404:19) at _run10000 (node_modules/@jest/core/build/cli/index.js:320:7) at runCLI (node_modules/@jest/core/build/cli/index.js:173:3) Test Suites: 1 failed, 1 total Tests: 1 failed, 1 total Snapshots: 0 total Time: 1.998 s Ran all test suites matching /theme-schema.test.js/i.Those individual fixes are documented in these commits:
Fix missing property object types for errors like:
Remove type conflict in fontWeight for this error:
Refactor fontSizes.fluid into non-union type for this error:
Fix core/archives non-property block having additionalProperties: false for this error:
Some additional fixes in Fix stylesProperties type and Fix styleProperties border direction properties that cropped up after the previous fixes, mostly addressing additional
propertiesobject types missing. This also fixes theborderautogenerated documentation fortop/bottom/left/right.Remove sub-properties from
core/archivein 3c32915 per this discussion.Testing Instructions
To test theme validation, run: