From 51e3f14eeeaaa62d45c5dc11f4eb0d589a7d3d77 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Mon, 20 Jan 2020 23:02:04 +0800 Subject: [PATCH] Restore coverage to 100% (#2372) --- src/execution/__tests__/variables-test.js | 12 ++ src/execution/values.js | 2 +- .../__tests__/mapAsyncIterator-test.js | 8 +- src/subscription/__tests__/subscribe-test.js | 5 +- src/type/__tests__/validation-test.js | 142 +++++++++++------- src/type/validate.js | 6 +- .../__tests__/findDeprecatedUsages-test.js | 14 ++ .../__tests__/getOperationAST-test.js | 14 +- 8 files changed, 134 insertions(+), 69 deletions(-) diff --git a/src/execution/__tests__/variables-test.js b/src/execution/__tests__/variables-test.js index 1963f6d59e..f0ce6253a5 100644 --- a/src/execution/__tests__/variables-test.js +++ b/src/execution/__tests__/variables-test.js @@ -1017,6 +1017,18 @@ describe('Execute: Handles inputs', () => { }; } + it('return all errors by default', () => { + const result = getVariableValues(schema, variableDefinitions, inputValue); + + expect(result).to.deep.equal({ + errors: [ + invalidValueError(0, 0), + invalidValueError(1, 1), + invalidValueError(2, 2), + ], + }); + }); + it('when maxErrors is equal to number of errors', () => { const result = getVariableValues( schema, diff --git a/src/execution/values.js b/src/execution/values.js index bc1df4665d..d284587350 100644 --- a/src/execution/values.js +++ b/src/execution/values.js @@ -50,8 +50,8 @@ export function getVariableValues( inputs: { +[variable: string]: mixed, ... }, options?: {| maxErrors?: number |}, ): CoercedVariableValues { - const maxErrors = options?.maxErrors; const errors = []; + const maxErrors = options?.maxErrors; try { const coerced = coerceVariableValues(schema, varDefNodes, inputs, error => { if (maxErrors != null && errors.length >= maxErrors) { diff --git a/src/subscription/__tests__/mapAsyncIterator-test.js b/src/subscription/__tests__/mapAsyncIterator-test.js index ea5f012bde..d498a3b137 100644 --- a/src/subscription/__tests__/mapAsyncIterator-test.js +++ b/src/subscription/__tests__/mapAsyncIterator-test.js @@ -273,7 +273,9 @@ describe('mapAsyncIterator', () => { } catch (e) { caughtError = e; } - expect(caughtError?.message).to.equal('Goodbye'); + + invariant(caughtError != null); + expect(caughtError.message).to.equal('Goodbye'); }); it('maps over thrown errors if second callback provided', async () => { @@ -294,8 +296,8 @@ describe('mapAsyncIterator', () => { }); const result = await doubles.next(); - expect(result.value).to.be.instanceof(Error); - expect(result.value?.message).to.equal('Goodbye'); + invariant(result.value instanceof Error); + expect(result.value.message).to.equal('Goodbye'); expect(result.done).to.equal(false); expect(await doubles.next()).to.deep.equal({ diff --git a/src/subscription/__tests__/subscribe-test.js b/src/subscription/__tests__/subscribe-test.js index eb4fb76536..45357218f2 100644 --- a/src/subscription/__tests__/subscribe-test.js +++ b/src/subscription/__tests__/subscribe-test.js @@ -8,6 +8,8 @@ import EventEmitter from 'events'; import { expect } from 'chai'; import { describe, it } from 'mocha'; +import invariant from '../../jsutils/invariant'; + import { parse } from '../../language/parser'; import { GraphQLError } from '../../error/GraphQLError'; @@ -142,7 +144,8 @@ async function expectPromiseToThrow(promise, message) { /* istanbul ignore next */ expect.fail('promise should have thrown but did not'); } catch (error) { - expect(error?.message).to.equal(message); + invariant(error instanceof Error); + expect(error.message).to.equal(message); } } diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index fd06476742..7dc138f29d 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -13,88 +13,92 @@ import { buildSchema } from '../../utilities/buildASTSchema'; import { GraphQLSchema } from '../schema'; import { GraphQLString } from '../scalars'; -import { GraphQLDirective } from '../directives'; import { validateSchema, assertValidSchema } from '../validate'; +import { GraphQLDirective, assertDirective } from '../directives'; import { type GraphQLNamedType, type GraphQLInputType, type GraphQLOutputType, GraphQLList, GraphQLNonNull, - GraphQLScalarType, GraphQLObjectType, GraphQLInterfaceType, GraphQLUnionType, GraphQLEnumType, GraphQLInputObjectType, + assertScalarType, + assertInterfaceType, + assertObjectType, + assertUnionType, + assertEnumType, + assertInputObjectType, } from '../definition'; -const SomeScalarType = new GraphQLScalarType({ name: 'SomeScalar' }); +const SomeSchema = buildSchema(` + scalar SomeScalar -const SomeInterfaceType = new GraphQLInterfaceType({ - name: 'SomeInterface', - fields: () => ({ f: { type: SomeObjectType } }), -}); + interface SomeInterface { f: SomeObject } -const SomeObjectType = new GraphQLObjectType({ - name: 'SomeObject', - fields: () => ({ f: { type: SomeObjectType } }), - interfaces: [SomeInterfaceType], -}); + type SomeObject implements SomeInterface { f: SomeObject } -const SomeUnionType = new GraphQLUnionType({ - name: 'SomeUnion', - types: [SomeObjectType], -}); + union SomeUnion = SomeObject -const SomeEnumType = new GraphQLEnumType({ - name: 'SomeEnum', - values: { - ONLY: {}, - }, -}); + enum SomeEnum { ONLY } -const SomeInputObjectType = new GraphQLInputObjectType({ - name: 'SomeInputObject', - fields: { - val: { type: GraphQLString, defaultValue: 'hello' }, - }, -}); + input SomeInputObject { val: String = "hello" } + + directive @SomeDirective on QUERY +`); + +const SomeScalarType = assertScalarType(SomeSchema.getType('SomeScalar')); +const SomeInterfaceType = assertInterfaceType( + SomeSchema.getType('SomeInterface'), +); +const SomeObjectType = assertObjectType(SomeSchema.getType('SomeObject')); +const SomeUnionType = assertUnionType(SomeSchema.getType('SomeUnion')); +const SomeEnumType = assertEnumType(SomeSchema.getType('SomeEnum')); +const SomeInputObjectType = assertInputObjectType( + SomeSchema.getType('SomeInputObject'), +); + +const SomeDirective = assertDirective(SomeSchema.getDirective('SomeDirective')); function withModifiers( - types: Array, + type: T, ): Array | GraphQLNonNull>> { return [ - ...types, - ...types.map(type => GraphQLList(type)), - ...types.map(type => GraphQLNonNull(type)), - ...types.map(type => GraphQLNonNull(GraphQLList(type))), + type, + GraphQLList(type), + GraphQLNonNull(type), + GraphQLNonNull(GraphQLList(type)), ]; } -const outputTypes = withModifiers([ - GraphQLString, - SomeScalarType, - SomeEnumType, - SomeObjectType, - SomeUnionType, - SomeInterfaceType, -]); - -const notOutputTypes = withModifiers([SomeInputObjectType]); - -const inputTypes = withModifiers([ - GraphQLString, - SomeScalarType, - SomeEnumType, - SomeInputObjectType, -]); - -const notInputTypes = withModifiers([ - SomeObjectType, - SomeUnionType, - SomeInterfaceType, -]); +const outputTypes: Array = [ + ...withModifiers(GraphQLString), + ...withModifiers(SomeScalarType), + ...withModifiers(SomeEnumType), + ...withModifiers(SomeObjectType), + ...withModifiers(SomeUnionType), + ...withModifiers(SomeInterfaceType), +]; + +const notOutputTypes: Array = [ + ...withModifiers(SomeInputObjectType), +]; + +const inputTypes: Array = [ + ...withModifiers(GraphQLString), + ...withModifiers(SomeScalarType), + ...withModifiers(SomeEnumType), + ...withModifiers(SomeInputObjectType), +]; + +const notInputTypes: Array = [ + ...withModifiers(SomeObjectType), + ...withModifiers(SomeUnionType), + ...withModifiers(SomeInterfaceType), +]; function schemaWithFieldType(type) { return new GraphQLSchema({ @@ -380,16 +384,40 @@ describe('Type System: A Schema must have Object root types', () => { ]); }); + it('rejects a Schema whose types are incorrectly typed', () => { + const schema = new GraphQLSchema({ + query: SomeObjectType, + // $DisableFlowOnNegativeTest + types: [{ name: 'SomeType' }, SomeDirective], + }); + expect(validateSchema(schema)).to.deep.equal([ + { + message: 'Expected GraphQL named type but got: { name: "SomeType" }.', + }, + { + message: 'Expected GraphQL named type but got: @SomeDirective.', + locations: [{ line: 14, column: 3 }], + }, + ]); + }); + it('rejects a Schema whose directives are incorrectly typed', () => { const schema = new GraphQLSchema({ query: SomeObjectType, // $DisableFlowOnNegativeTest - directives: ['SomeDirective'], + directives: [null, 'SomeDirective', SomeScalarType], }); expect(validateSchema(schema)).to.deep.equal([ + { + message: 'Expected directive but got: null.', + }, { message: 'Expected directive but got: "SomeDirective".', }, + { + message: 'Expected directive but got: SomeScalar.', + locations: [{ line: 2, column: 3 }], + }, ]); }); }); diff --git a/src/type/validate.js b/src/type/validate.js index 2c13e51865..010ed3549e 100644 --- a/src/type/validate.js +++ b/src/type/validate.js @@ -204,7 +204,7 @@ function validateTypes(context: SchemaValidationContext): void { if (!isNamedType(type)) { context.reportError( `Expected GraphQL named type but got: ${inspect(type)}.`, - type?.astNode, + type.astNode, ); continue; } @@ -354,7 +354,7 @@ function validateTypeImplementsInterface( `Interface field ${iface.name}.${fieldName} expects type ` + `${inspect(ifaceField.type)} but ${type.name}.${fieldName} ` + `is type ${inspect(typeField.type)}.`, - [ifaceField.astNode?.type, typeField.astNode?.type], + [ifaceField.astNode.type, typeField.astNode.type], ); } @@ -381,7 +381,7 @@ function validateTypeImplementsInterface( `expects type ${inspect(ifaceArg.type)} but ` + `${type.name}.${fieldName}(${argName}:) is type ` + `${inspect(typeArg.type)}.`, - [ifaceArg.astNode?.type, typeArg.astNode?.type], + [ifaceArg.astNode.type, typeArg.astNode.type], ); } diff --git a/src/utilities/__tests__/findDeprecatedUsages-test.js b/src/utilities/__tests__/findDeprecatedUsages-test.js index 42c96a5492..44c4548857 100644 --- a/src/utilities/__tests__/findDeprecatedUsages-test.js +++ b/src/utilities/__tests__/findDeprecatedUsages-test.js @@ -30,6 +30,20 @@ describe('findDeprecatedUsages', () => { expect(errors.length).to.equal(0); }); + it('should ignore unknown stuff', () => { + const errors = findDeprecatedUsages( + schema, + parse(` + { + unknownField(unknownArg: UNKNOWN_VALUE) + normalField(enumArg: UNKNOWN_VALUE) + } + `), + ); + + expect(errors.length).to.equal(0); + }); + it('should report usage of deprecated fields', () => { const errors = findDeprecatedUsages( schema, diff --git a/src/utilities/__tests__/getOperationAST-test.js b/src/utilities/__tests__/getOperationAST-test.js index 6367eda9a0..8bc4c1646a 100644 --- a/src/utilities/__tests__/getOperationAST-test.js +++ b/src/utilities/__tests__/getOperationAST-test.js @@ -32,7 +32,8 @@ describe('getOperationAST', () => { const doc = parse(` { field } mutation Test { field } - subscription TestSub { field }`); + subscription TestSub { field } + `); expect(getOperationAST(doc)).to.equal(null); }); @@ -40,15 +41,19 @@ describe('getOperationAST', () => { const doc = parse(` query TestQ { field } mutation TestM { field } - subscription TestS { field }`); + subscription TestS { field } + `); expect(getOperationAST(doc)).to.equal(null); }); it('Does not get misnamed operation', () => { const doc = parse(` + { field } + query TestQ { field } mutation TestM { field } - subscription TestS { field }`); + subscription TestS { field } + `); expect(getOperationAST(doc, 'Unknown')).to.equal(null); }); @@ -56,7 +61,8 @@ describe('getOperationAST', () => { const doc = parse(` query TestQ { field } mutation TestM { field } - subscription TestS { field }`); + subscription TestS { field } + `); expect(getOperationAST(doc, 'TestQ')).to.equal(doc.definitions[0]); expect(getOperationAST(doc, 'TestM')).to.equal(doc.definitions[1]); expect(getOperationAST(doc, 'TestS')).to.equal(doc.definitions[2]);