Remove remaining JSON schema usage from schema definition specs#1250
Closed
jwils wants to merge 1 commit into
Closed
Remove remaining JSON schema usage from schema definition specs#1250jwils wants to merge 1 commit into
jwils wants to merge 1 commit into
Conversation
As noted on #1224, mentions of JSON schema in `elasticgraph-schema_definition/spec` should approach zero now that the JSON schema logic lives in `elasticgraph-json_ingestion`: - The graphql_schema, datastore_config, and runtime_metadata spec supports now run schemas without any extension modules, and the `json_schema` calls that existed only to satisfy the extension's scalar validation are gone. - The scalar `json_schema` requirement test (duplicated by the json_ingestion suite) is deleted, and the `long`/`unsigned_long` placeholder-inference tests that depend on JSON schema bounds moved to the json_ingestion suite (along with the built-in-scalar placeholder map, which differs with the extension loaded). - The reserved-type-name test now exercises the core `reserved_type_names` mechanism directly; the `ElasticGraphEventEnvelope` reservation is already covered by the json_ingestion suite. - `rake_tasks_spec` runs its synthetic schemas without the extension and no longer asserts on JSON schema artifacts (covered by json_ingestion's integration spec). A new short-diff test keeps `truncate_diff` fully covered. The tests that evaluate the repo's own `config/schema.rb` still load the extension, since that schema is a JSON ingestion application. The remaining mentions are the `define_schema` test-support seam (which exists for optional ingestion extensions) and JSON-the-format documentation text.
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.
Why
On #1224, @myronmarston noted that mentions of "JSON schema" in
elasticgraph-schema_definition/specshould approach zero as part of the extraction, but his grep showed many remained.What
json_schemacalls that existed only to satisfy the JSON ingestion extension's scalar validation are removed.json_schema" test (duplicated by the json_ingestion suite) is deleted, and thelong/unsigned_longplaceholder-inference tests that depend on JSON schema bounds moved toscalar_type_extension_specinelasticgraph-json_ingestion(along with a built-in-scalar placeholder map test, sinceJsonSafeLong's placeholder differs with the extension loaded).reserved_type_namesmechanism directly; theElasticGraphEventEnvelopereservation is already covered by the json_ingestion suite.rake_tasks_specruns its synthetic schemas without the extension and no longer asserts on JSON schema artifacts (covered by json_ingestion'sschema_artifact_manager_extension_spec). A new short-diff test keepsSchemaArtifactManager#truncate_difffully covered.json_schema_spec.What intentionally remains: the rake-task tests that evaluate the repo's own
config/schema.rbkeep the extension (that schema is a JSON ingestion application), thedefine_schematest-support seam tests (the seam exists for optional ingestion extensions), and documentation text about JSON-the-format (JsonSafeLongetc.).Verification
script/run_gem_specs elasticgraph-schema_definitionandscript/run_gem_specs elasticgraph-json_ingestion(both 100% line + branch coverage)script/type_checkscript/quick_buildStack
Current PR is marked with
->.