Skip to content
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

Merged
merged 2 commits into from
Apr 6, 2018

Conversation

IvanGoncharov
Copy link
Member

Ideally these should detect the exact set of schema validation errors and we should write the test so that each test introduces only the validation error it expects to catch

@leebyron Done

Copy link
Contributor

@leebyron leebyron left a 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}.`,
Copy link
Contributor

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?

Copy link
Member Author

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}.`,
Copy link
Contributor

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}.`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants