Repository navigation
test(schema): check the Google schema against the loader - #555
Merged
Merged
Conversation
merr1988
force-pushed
the
feat/schema/model-agreement
branch
from
October 23, 2026 17:50
feb5aed to
4e0c47c
Compare
merr1988
force-pushed
the
feat/schema/model-agreement
branch
from
October 23, 2026 17:56
4e0c47c to
0ea1a0c
Compare
feixufeixu
approved these changes
Oct 23, 2026
feixufeixu
left a comment
feixufeixu
left a comment
Collaborator
There was a problem hiding this comment.
Approving. I left five required changes, each with its exact fix, and a few optional ones.
Adds the agreement tests b/568049508 asks for. Each case is one document and its verdict, checked against the published schema and against what push accepts. A case where the two disagree fails. - Model documents (114 cases): the schema root against the loader with bindingOptional, then resolveModelDeployments, validateDeployments and validatePushRequirements with targetOptional on the model with no deployments, as a Knowledge Catalog push runs them. - Profile files (49 cases): #/$defs/ProfileDocument against the layout's profile-name rules (nameProblem, reservedProfileNameError), loadProfileFile and validateProfileCompleteness. The cases cover the bug's test list: valid minimal and full models, invalid shapes, constraints, deployments that use the model file's own bindings, names, key and join columns, executors, metrics_exclude, fields_exclude, and one valid and one malformed case per source and target form. Both tables share one runner and one source-case builder. Schema fixes found by the cases: an executor coordinate and a sql statement may not be blank, as push requires. Known disagreement: the loader accepts an empty name or description on actions, action parameters and constraints; the schema does not. Those six cases name b/577102892 and expect the disagreement, so they fail once the loader is fixed. Bug: b/568049508
- The model runner also runs undeclaredProfileError and validateInlineDialects, and the profile runner validates the model with the profile applied, as push does. - Known differences that no JSON Schema can state: a metric reading a field with no expression, and a ratio metric on a BigQuery deployment. - New cases: a deployment profile with a space, a blank mcp tool, rest endpoint and grpc method, and a profile action with a blank server. - A profile entity with no primary_key is checked against the schema alone, since push decides it from the model file. - One columnCases helper serves both tables; fields_exclude '*' no longer leaves a field unbound. Bug: b/568049508
merr1988
force-pushed
the
feat/schema/model-agreement
branch
from
October 23, 2026 23:14
6cbf37a to
f879ae3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of b/568049508, after #551.
What
Agreement tests: each case is one document and whether it is valid. It is checked against the published schema and against what push accepts. A case where the two disagree fails.
fromDocument({bindingOptional: true}), thenresolveModelDeployments,undeclaredProfileError,validateInlineDialects,validateDeploymentsandvalidatePushRequirements({targetOptional: true})on the model with no deployments, as the Knowledge Catalog push runs them.#/$defs/ProfileDocumentvs. the layout's profile-name rules (nameProblem,reservedProfileNameError), thenloadProfileFile,validateProfileCompleteness, andvalidatePushRequirementson the model with the profile applied.The cases cover the bug's test list:
metrics_excludeandfields_exclude;Both tables share one runner (
describeAgreement) and one source-case builder (sourceCases).Schema change
Found by the cases: an executor coordinate (for example
server: " ") and a SQL statement may not be blank, as push already requires. Both use one newNonBlankTextdefinition.Known disagreements
Two rules about what a metric's SQL reads, which no JSON Schema can state: a metric reading a field with no expression, and a ratio metric on a BigQuery deployment. Each case names the reason and expects the disagreement.
b/577102892
The loader accepts an empty name or description on actions, action parameters and constraints. The schema rejects them, as the bug specifies. The six cases name b/577102892 and expect the disagreement, so they fail once the loader is fixed and the entries are removed.
Testing
tscclean. Semantic suite: 2054 pass.maxProperties, the backtick inColumnName, or the blank-statement rule each fails exactly the matching cases.