test: characterize Client#parse handling of inline named fragments#81
Open
rellampec wants to merge 1 commit into
Open
Conversation
Pin current behavior of locally-defined named fragments (a 'fragment Name on Type' spread via '...Name' within the same document), transitive local fragments, and the source_document vs sliced document distinction. These were only exercised incidentally alongside constant-path spreads; isolating them guards a future refactor of the fragment-resolution step in parse.
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.
Pin current behavior of locally-defined named fragments (a 'fragment Name on Type' spread via '...Name' within the same document), transitive local fragments, and the source_document vs sliced document distinction. These were only exercised incidentally alongside constant-path spreads; isolating them guards a future refactor of the fragment-resolution step in parse.
What this does
Adds characterization tests that pin the current behaviour of
Client#parsewhen a document contains an inlinenamed fragment — a
fragment Name on Type { … }definition spread via...Namefrom within the sameparsestring(as opposed to a fragment bound to a Ruby constant).
New file:
test/test_parse_fragment_characterization.rb— 3 tests / 18 assertions, Minitest, passing.It pins three behaviours that are currently exercised only incidentally:
...Namespread referring to a fragment defined in the same document(with no matching Ruby constant) is resolved locally: the definition is kept and renamed with the document's constant
path, the spread is rewritten to the renamed name, and no
ValidationErroris raised.chain, and the operation's sliced
documentcarries the full transitive dependency set.source_documentvs sliceddocument— for a multi-operation document, each operation'sdocumentis slicedto just that operation + its own fragment dependencies, while
source_documentretains every sibling definition.Why
This is test-only — no behaviour change. It's a safety net placed before a follow-up that will tighten fragment-spread matching in
Client#parse.Today the spread/definition detection uses an unanchored
str.match(/fragment\s*#{const_name}/), which can misfire on prefix collisions (e.g....UserFieldsmatchingfragment UserFieldsExtended) and even onfragmentUserFields. A planned change will anchor that tofragment\s+Name\s+on\b. Pinning the surrounding behaviour first means that refactor can't silently change any of the guarantees above.Worth noting: an earlier attempt to "improve spread constant matching" was reverted (
b5a4612), so characterizing the current behaviour before touching it again seemed prudent.Relationship to other PRs
master, clean diff.