Show schema definition order differences - #1355
Conversation
a5aa001 to
ef51bc3
Compare
jwils
changed the title
myronmarston
left a comment
myronmarston
left a comment
There was a problem hiding this comment.
I don't really get what benefit this PR provides. It changes how we assert that every definition order produces the same SDL, but does it catch any additional cases or produce better failure output?
I believe the existing test already fully enforces consistently ordered output. But maybe I'm missing something...
| # Note: when this spec was written, these first 2 orderings (where `Component` came first) caused infinite recursion. | ||
| # At the time, having `Component` come first caused an issue because it caused `ComponentEdge` (with it's | ||
| # `node: Component` field) and `ComponentConnection` (with its `nodes: [Component!]!` field) to be generated | ||
| # before, `Widget.components` was processed, leading to additional field references to the `Component` type. |
There was a problem hiding this comment.
| # before `Widget.components` was processed, leading to additional field references to the `Component` type. |
(I just noticed this while reviewing the overall test...).
| reference_order, reference_result = results_by_definition_order.first | ||
|
|
||
| # Also compare the first and last, so that if there are multiple we get a diff showing how they differ. | ||
| expect(uniq_results_for_each_ordering.first).to eq uniq_results_for_each_ordering.last | ||
| # Compare the raw generated SDL rather than normalizing it through graphql-ruby. Schema definition | ||
| # order must not affect field order or any other part of the generated schema artifact. | ||
| results_by_definition_order.drop(1).each do |definition_order, result| |
There was a problem hiding this comment.
| reference_order, reference_result = results_by_definition_order.first | |
| # Also compare the first and last, so that if there are multiple we get a diff showing how they differ. | |
| expect(uniq_results_for_each_ordering.first).to eq uniq_results_for_each_ordering.last | |
| # Compare the raw generated SDL rather than normalizing it through graphql-ruby. Schema definition | |
| # order must not affect field order or any other part of the generated schema artifact. | |
| results_by_definition_order.drop(1).each do |definition_order, result| | |
| (reference_order, reference_result), *remaining_results = results_by_definition_order | |
| # Compare the raw generated SDL rather than normalizing it through graphql-ruby. Schema definition | |
| # order must not affect field order or any other part of the generated schema artifact. | |
| remaining_results.each do |definition_order, result| |
I was initially confused by the drop(1). Note: I haven't tried this, so it may not quite work as I've written it. Pattern matching syntax might be needed to make it work.
|
Doesn't do anything yet, but I am trying to reproduce https://github.com/block/elasticgraph/actions/runs/31920698193/job/95100012949?pr=1305 with more debug info. I think a flaky test, but not enough info on the failure? Let me know if you have any suggestions. Otherwise I may stop worrying about it since I can't seem to reproduce it. |
|
jruby/jruby#9591 is the root cause I believe. |
2 participants
Why
An intermittent JRuby failure proves that schema definition order can produce two different GraphQL outputs, but the current assertion stops after counting unique results and hides the actual SDL difference. Capture an actionable diff before choosing a production fix.
What
Risk Assessment
Very low. This changes one test assertion and no production behavior; the same flake may recur, but its next failure will identify the divergent ordering and output.
Validation
script/lintContext
Observed while validating #1305.