-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Match exact set of errors in schema validation tests #1307
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! It helped highlight some issues along the way, as I thought it might
message: `The type of BadInterface.badField must be Output Type but got: ${Number}.`, | ||
}, | ||
{ | ||
message: `Expected GraphQL named type but got: ${Number}.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one feels low value and duplicative of the error below it, don't you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leebyron Agree 👍
However I don't see how validate
can distinguish beetween those two cases:
validateSchema(new GraphQLSchema({
queryType: Number,
}));
validateSchema(new GraphQLSchema({
queryType: new GraphQLObjectType({
name: 'Foo',
fields: { bar: { type: GraphQLString } },
}),
types: [Number],
}));
In the first case Expected GraphQL named type but got: ${Number}.
is redundant but in second one it's the only error that would be reported.
Can you suggest how to solve this?
message: `The type of BadObject.badField(badArg:) must be Input Type but got: ${Number}.`, | ||
}, | ||
{ | ||
message: `Expected GraphQL named type but got: ${Number}.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another example. This is an opportunity to improve the quality of these errors in the future
message: `The type of BadInputObject.badField must be Input Type but got: ${Number}.`, | ||
}, | ||
{ | ||
message: `Expected GraphQL named type but got: ${Number}.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
@leebyron Done