Skip to content

test: Keep the #926 keyword dispatch map in sync and fix its tests - #993

Open
Roman3349 wants to merge 3 commits into
jsonrainbow:fix/920-avoid-unconditional-constraint-instantiationfrom
Roman3349:fix/926-keyword-dispatch-follow-up
Open

Roman3349 wants to merge 3 commits into
jsonrainbow:fix/920-avoid-unconditional-constraint-instantiationfrom
Roman3349:fix/926-keyword-dispatch-follow-up

Conversation

@Roman3349

@Roman3349 Roman3349 commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Description

Follow-up to #926, targeting its branch. It addresses the review comments on #926:

  • KeywordSchemaPropertiesTest: keeps KEYWORD_SCHEMA_PROPERTIES in sync with the keyword constraints. For every keyword in each draft factory's constraintMap, it compares the schema properties in the first condition of the constraint's check() with the properties the dispatcher checks. A keyword whose schema property differs from its name and is missing from the map would otherwise silently never run.
    • It passes for all 97 keywords of Draft-06, Draft-07 and Draft 2019-09. Draft 2019-09 content is skipped, because its check() is a no-op.
    • Merged with feat: Support $recursiveRef in Draft 2019-09 #984, it fails on Draft2019Constraint: recursiveRef until 'recursiveRef' => ['$recursiveRef'] is added to the map. With that entry, the whole suite passes.
  • Dispatcher tests fixed:
    • The test named after dependentSchemas and dependentRequired only used dependentRequired (the Copilot review comment). Both are now covered separately.
    • The tests mocked the base JsonSchema\Constraints\Factory, but the draft constraints always run with their own draft factory and may call methods only that factory has. Merged with feat: Support $recursiveRef in Draft 2019-09 #984, all Draft 2019-09 dispatcher tests failed with Call to undefined method Mock_Factory_…::getDynamicScope(). The tests now mock the draft factory.
  • KEYWORD_SCHEMA_PROPERTIES is declared at the top of the class instead of between check() and checkForKeyword().

Related Issue

Related to #920 and #926

Type of Change

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the code style of this project
  • I have added tests that prove my fix is effective or that my feature works
  • All new and existing tests pass
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional Notes

Verified on this branch merged with the current main (10162 tests pass), and additionally merged with #984 plus the recursiveRef map entry (10186 tests pass).

A correction to the BC concern I raised on #926: custom constraint classes registered with Factory::setConstraintClass() on the factory passed to the Validator never reach strict mode at all, because each Draft*Constraint creates its own draft factory and only takes the schema storage, URI retriever and config from the given one. So the gating only affects a subclass of a Draft*Constraint that registers a custom keyword on the draft factory, and only when that keyword's name differs from its schema property. That is narrow enough not to block #926.

…ints

The draft constraints only dispatch a keyword when one of the schema
properties mapped to it is present. A keyword whose schema property
differs from its name and is missing from KEYWORD_SCHEMA_PROPERTIES is
silently never checked.

The test compares, for every keyword in each draft factory, the schema
properties in the first condition of the constraint's check() with the
properties the dispatcher checks.

Signed-off-by: Roman Ondráček <mail@romanondracek.cz>
Signed-off-by: Roman Ondráček <mail@romanondracek.cz>
- The test named after dependentSchemas and dependentRequired only used
  dependentRequired, so dependentSchemas was not covered. Both are now
  covered separately.
- The draft constraints always run with their own draft factory, and may
  call methods only that factory has, such as getDynamicScope() needed
  for $recursiveRef in Draft 2019-09. A mock of the base factory lacks
  them, so the tests mock the draft factory.

Signed-off-by: Roman Ondráček <mail@romanondracek.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants