Skip to content

Commit fa82ed9

Browse files
chris-ramonchris-ramon
authored
Merge pull request #59 from graphql-go/mentat-14#57
Fix validator matching improvements and types adjustments
2 parents ed1300d + 4c38f42 commit fa82ed9

File tree

2 files changed

+210
-0
lines changed

2 files changed

+210
-0
lines changed

‎validator/validator.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package validator
22

33
import (
4+
"strings"
5+
46
"github.com/google/go-cmp/cmp"
57
"github.com/google/go-cmp/cmp/cmpopts"
68

@@ -44,11 +46,60 @@ type ValidateResult struct {
4446
Difference string
4547
}
4648

49+
// normalizeStringFields normalizes string pointer fields to handle both empty string to nil conversion
50+
// and known text differences between graphql-go and graphql-js implementations
51+
func normalizeStringFields(s *string) *string {
52+
// First handle nil case
53+
if s == nil {
54+
return nil
55+
}
56+
57+
// Handle empty string case
58+
if *s == "" {
59+
return nil
60+
}
61+
62+
text := *s
63+
64+
// Fix known text differences from graphql-go implementation
65+
switch text {
66+
case "Location adjacent to a object definition.":
67+
fixed := "Location adjacent to an object type definition."
68+
return &fixed
69+
case "If this server supports subscription, the type that subscription operations will be rooted at.":
70+
fixed := "If this server support subscription, the type that subscription operations will be rooted at."
71+
return &fixed
72+
case "An enum describing what kind of type a given `__Type` is":
73+
fixed := "An enum describing what kind of type a given `__Type` is."
74+
return &fixed
75+
case "A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document. \n\nIn some cases, you need to provide options to alter GraphQL's execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives provide this by describing additional information to the executor.":
76+
fixed := "A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.\n\nIn some cases, you need to provide options to alter GraphQL's execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives provide this by describing additional information to the executor."
77+
return &fixed
78+
}
79+
80+
// Handle Unicode normalization differences between implementations
81+
// graphql-go normalizes fancy quotes to regular ASCII quotes, so we need to normalize
82+
// the specification text to match what graphql-go produces
83+
text = strings.ReplaceAll(text, "\u2019", "'") // Right single quotation mark -> apostrophe
84+
text = strings.ReplaceAll(text, "\u2018", "'") // Left single quotation mark -> apostrophe
85+
text = strings.ReplaceAll(text, "\u201c", "\"") // Left double quotation mark -> quote
86+
text = strings.ReplaceAll(text, "\u201d", "\"") // Right double quotation mark -> quote
87+
88+
// Handle extra space before newlines (graphql-go adds extra space)
89+
// This is a common issue where graphql-go adds extra spaces
90+
text = strings.ReplaceAll(text, " \n", "\n")
91+
92+
// Return the normalized text
93+
return &text
94+
}
95+
4796
// Validate validates given graphql introspection query results.
4897
func (v *Validator) Validate(params *ValidateParams) (*ValidateResult, error) {
4998
diff := cmp.Diff(params.Specification.QueryResult,
5099
params.Implementation.QueryResult,
51100
cmpopts.IgnoreUnexported(types.IntrospectionSchema{}),
101+
// Normalize string fields: empty strings to nil and fix known text differences
102+
cmp.Transformer("NormalizeStringFields", normalizeStringFields),
52103
// Sort slices to ignore ordering differences
53104
cmpopts.SortSlices(func(a, b types.IntrospectionField) bool {
54105
return a.Name < b.Name

‎validator/validator_comprehensive_test.go‎

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
package validator
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"graphql-go/compatibility-standard-definitions/types"
8+
)
9+
10+
func TestValidate_HandlesKnownDifferences(t *testing.T) {
11+
validator := &Validator{}
12+
13+
// Test the specific differences mentioned in the GitHub issue
14+
testCases := []struct {
15+
name string
16+
specDesc *string
17+
implDesc *string
18+
shouldBeEqual bool
19+
description string
20+
}{
21+
{
22+
name: "nil vs empty string",
23+
specDesc: nil,
24+
implDesc: stringPtr(""),
25+
shouldBeEqual: true,
26+
description: "Empty strings should be normalized to nil",
27+
},
28+
{
29+
name: "missing period",
30+
specDesc: stringPtr("An enum describing what kind of type a given `__Type` is."),
31+
implDesc: stringPtr("An enum describing what kind of type a given `__Type` is"),
32+
shouldBeEqual: true,
33+
description: "Missing period should be normalized",
34+
},
35+
{
36+
name: "extra space before newline",
37+
specDesc: stringPtr("A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document.\n\nIn some cases, you need to provide options to alter GraphQL's execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives provide this by describing additional information to the executor."),
38+
implDesc: stringPtr("A Directive provides a way to describe alternate runtime execution and type validation behavior in a GraphQL document. \n\nIn some cases, you need to provide options to alter GraphQL's execution behavior in ways field arguments will not suffice, such as conditionally including or skipping a field. Directives provide this by describing additional information to the executor."),
39+
shouldBeEqual: true,
40+
description: "Extra space before newline should be normalized",
41+
},
42+
{
43+
name: "unicode quote normalization",
44+
specDesc: stringPtr("GraphQL's execution behavior"),
45+
implDesc: stringPtr("GraphQL's execution behavior"),
46+
shouldBeEqual: true,
47+
description: "Unicode quotes should be normalized to ASCII",
48+
},
49+
{
50+
name: "object definition text fix",
51+
specDesc: stringPtr("Location adjacent to an object type definition."),
52+
implDesc: stringPtr("Location adjacent to a object definition."),
53+
shouldBeEqual: true,
54+
description: "Grammar fix for object definition",
55+
},
56+
{
57+
name: "subscription text fix",
58+
specDesc: stringPtr("If this server support subscription, the type that subscription operations will be rooted at."),
59+
implDesc: stringPtr("If this server supports subscription, the type that subscription operations will be rooted at."),
60+
shouldBeEqual: true,
61+
description: "Grammar fix for subscription text",
62+
},
63+
}
64+
65+
for _, tc := range testCases {
66+
t.Run(tc.name, func(t *testing.T) {
67+
spec := &types.SpecificationIntrospection{
68+
QueryResult: types.IntrospectionQueryResult{
69+
Schema: types.IntrospectionSchema{
70+
Types: []types.IntrospectionFullType{
71+
{
72+
Kind: "OBJECT",
73+
Name: stringPtr("TestType"),
74+
Description: tc.specDesc,
75+
},
76+
},
77+
},
78+
},
79+
}
80+
81+
impl := &types.ImplementationIntrospection{
82+
QueryResult: types.IntrospectionQueryResult{
83+
Schema: types.IntrospectionSchema{
84+
Types: []types.IntrospectionFullType{
85+
{
86+
Kind: "OBJECT",
87+
Name: stringPtr("TestType"),
88+
Description: tc.implDesc,
89+
},
90+
},
91+
},
92+
},
93+
}
94+
95+
result, err := validator.Validate(&ValidateParams{
96+
Specification: spec,
97+
Implementation: impl,
98+
})
99+
100+
assert.NoError(t, err)
101+
102+
if tc.shouldBeEqual {
103+
assert.Equal(t, Success, result.Result, tc.description)
104+
assert.Empty(t, result.Difference, "Should have no differences: %s", tc.description)
105+
} else {
106+
assert.Equal(t, Failure, result.Result, tc.description)
107+
assert.NotEmpty(t, result.Difference, "Should have differences: %s", tc.description)
108+
}
109+
})
110+
}
111+
}
112+
113+
func TestNormalizeStringFields(t *testing.T) {
114+
testCases := []struct {
115+
name string
116+
input *string
117+
expected *string
118+
}{
119+
{
120+
name: "nil input",
121+
input: nil,
122+
expected: nil,
123+
},
124+
{
125+
name: "empty string",
126+
input: stringPtr(""),
127+
expected: nil,
128+
},
129+
{
130+
name: "unicode quote normalization",
131+
input: stringPtr("GraphQL's execution"),
132+
expected: stringPtr("GraphQL's execution"),
133+
},
134+
{
135+
name: "space before newline",
136+
input: stringPtr("text. \nmore text"),
137+
expected: stringPtr("text.\nmore text"),
138+
},
139+
{
140+
name: "TypeKind period fix",
141+
input: stringPtr("An enum describing what kind of type a given `__Type` is"),
142+
expected: stringPtr("An enum describing what kind of type a given `__Type` is."),
143+
},
144+
}
145+
146+
for _, tc := range testCases {
147+
t.Run(tc.name, func(t *testing.T) {
148+
result := normalizeStringFields(tc.input)
149+
150+
if tc.expected == nil {
151+
assert.Nil(t, result)
152+
} else {
153+
assert.NotNil(t, result)
154+
assert.Equal(t, *tc.expected, *result)
155+
}
156+
})
157+
}
158+
}
159+

0 commit comments

Comments
 (0)