Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Improve diagnostics tests #236

Merged
merged 5 commits into from
Jun 3, 2018
Merged

Improve diagnostics tests #236

merged 5 commits into from
Jun 3, 2018

Conversation

divyenduz
Copy link
Contributor

  1. Fix typo in language service test case

  2. Add more test cases for diagnostics

@@ -33,7 +33,7 @@ describe('GraphQLLanguageService', () => {

it('runs diagnostic service as expected', async () => {
const diagnostics = await languageService.getDiagnostics(
'qeury',
'query',
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a typo - as you see in line 39, this should expect a diagnostics error. Did this test pass with this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, yes the tests passed for this change, it because with query it yields the following syntax error:

[ { severity: 1,
    message: 'Syntax Error: Expected {, found <EOF>',
    source: 'GraphQL: Syntax',
    range: Range { containsPosition: [Function], start: [Object], end: [Object] } } ]

and with qeury it yields the following error:

[ { severity: 1,
    message: 'Syntax Error: Unexpected Name "qeury"',
    source: 'GraphQL: Syntax',
    range: Range { containsPosition: [Function], start: [Object], end: [Object] } } ]

So, the errors are different but since we are only checking for the error count, the test passes.

Reverted it to qeury and added a new assertion to make it clearer that it is not a typo and make the test more robust.

expect(errors.length).to.equal(0);
});

it('returns no errors for valid query', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated with above


it('returns no errors for valid query with aliases', () => {
const errors = getDiagnostics(
'query { superHero: hero { superName :name } }',
Copy link
Contributor

@asiandrummer asiandrummer Jun 2, 2018

Choose a reason for hiding this comment

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

nit: A better test case would also be:

'query { superHero: hero { superName: name } superHero2: hero { superName2: name } }'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

const errors = getDiagnostics(
`
type Human implements Character {
SyntaxError
Copy link
Contributor

Choose a reason for hiding this comment

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

wat, not sure I understand SyntaxError as a field - maybe I'm not caught up with the current GraphQL spec. Could you elaborate here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops! my bad, I wanted to show that this is a syntax error i.e. type name without a named type definition. Using camel case was wrong, changing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used a verbose field_without_type_is_a_syntax_error for field name. Please let me know if I should change it :)

@asiandrummer asiandrummer merged commit f37d147 into graphql:master Jun 3, 2018
@asiandrummer
Copy link
Contributor

Thanks!

@schickling schickling mentioned this pull request Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants