Skip to content

Add registration tests for the validator and composer rule lists - #10203

Merged
glen-84 merged 2 commits into
mainfrom
gai/rule-registration-tests
Aug 6, 2026
Merged

glen-84 merged 2 commits into
mainfrom
gai/rule-registration-tests

Conversation

@glen-84

@glen-84 glen-84 commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

  • Rule tests inject the rule under test directly, so a rule's own tests stay green when it is dropped from the list that actually runs it. Across the four lists only five of the 97 rules were pinned at all, incidentally, by composer-level tests that assert a specific error code.
  • Four tests now assert each registered set in order: SchemaValidator.AddDefaultRules (20 rules), and SchemaComposer's source-schema (37), pre-merge (20), and post-merge (20) lists. Order is part of the assertion because handler dispatch follows registration order.
  • To reach the lists, the three composer lists become internal static properties carrying the literal, so there is one member per list rather than a field plus an accessor, and SchemaValidator gains an internal Rules accessor with an InternalsVisibleTo entry for its test project.

Test plan

  • Six mutations confirm the tests bite: removing a rule from each of the four lists, adding a duplicate rule, and swapping two adjacent rules in AddDefaultRules.
  • Full suites run for Types.Validation (142) and Fusion.Composition (743).

Copilot AI review requested due to automatic review settings August 6, 2026 07:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens safety around schema validation/composition by adding tests that pin the registered rule lists (and their order), preventing rules from being silently dropped from the real execution pipelines.

Changes:

  • Add assertion tests that verify the exact rule registration sets and order for SchemaValidator defaults and SchemaComposer’s source/pre-merge/post-merge rule lists.
  • Expose SchemaComposer rule lists as internal static properties so tests can assert the literal registered sequences.
  • Add an internal accessor for SchemaValidator’s rules and grant test-assembly access via InternalsVisibleTo.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/HotChocolate/Fusion/test/Fusion.Composition.Tests/SchemaComposerTests.cs Adds tests asserting the registered composer rule lists and order.
src/HotChocolate/Fusion/src/Fusion.Composition/SchemaComposer.cs Exposes composer rule lists as internal properties and uses them at call sites.
src/HotChocolate/Core/test/Types.Validation.Tests/SchemaValidatorTests.cs Adds a test asserting the default schema validator rule registration/order.
src/HotChocolate/Core/src/Types.Validation/SchemaValidator.cs Adds an internal accessor exposing the validator’s configured rules for tests.
src/HotChocolate/Core/src/Types.Validation/HotChocolate.Types.Validation.csproj Adds InternalsVisibleTo for the Types.Validation test project.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/HotChocolate/Core/src/Types.Validation/SchemaValidator.cs
@github-actions

github-actions Bot commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Patch coverage

100.0% of changed lines covered (7/7)

File Covered Changed Patch %
…/HotChocolate/Core/src/Types.Validation/SchemaValidator.cs 1 1 100.0% 🟢
…/HotChocolate/Fusion/src/Fusion.Composition/SchemaComposer.cs 6 6 100.0% 🟢

Project coverage: 54.4% (242433/445718 lines)

@glen-84
glen-84 merged commit d77da5c into main Aug 6, 2026
289 of 291 checks passed
@glen-84
glen-84 deleted the gai/rule-registration-tests branch August 6, 2026 08:33
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