From c823ef1911d4d2ea731a471bfad635a1123db06a Mon Sep 17 00:00:00 2001 From: Oryan Date: Sun, 18 Oct 2026 17:42:57 -0400 Subject: [PATCH] Test the warnings for tolerated schema problems through a shared log-capture helper UnmappedInputFieldsTest's withWarnings moves to TestUtils and listens on the library's loggers instead of the root logger. The tests for unused types, allowUnimplementedResolvers, repeated enum values, interface fields and union members, extensions of a different kind and repeated non-repeatable directives now assert the exact warnings. --- .../tools/MissingFieldResolverTest.kt | 27 ++++ .../graphql/kickstart/tools/TestUtils.kt | 21 +++ .../kickstart/tools/TypeExtensionTest.kt | 149 ++++++++++-------- .../tools/UnmappedInputFieldsTest.kt | 21 --- 4 files changed, 134 insertions(+), 84 deletions(-) diff --git a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt index c3727952..a4962f67 100644 --- a/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/MissingFieldResolverTest.kt @@ -29,6 +29,33 @@ class MissingFieldResolverTest { .makeExecutableSchema() } + @Test + fun `should warn about each missing field when unimplemented resolvers are allowed`() { + val (schema, warnings) = withWarnings { + SchemaParser.newParser() + .schemaString( + """ + type Query { + implementedField(input: String): String + missingField(input: Int): Int + } + """ + ) + .resolvers(object : GraphQLQueryResolver { + fun implementedField(input: Optional) = input.toString() + }) + .options(SchemaParserOptions.newOptions().allowUnimplementedResolvers(true).build()) + .build() + .makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "Option 'allowUnimplementedResolvers' should only be set to true during development, as it can cause schema errors to be moved " + + "to query time instead of schema creation time. Make sure this is turned off in production.", + "Missing resolver for field: ${schema.queryType.getFieldDefinition("missingField").definition}" + )) + } + @Test @Suppress("DEPRECATION") fun `should call missing resolver data fetcher if provided`() { diff --git a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt index 7aa9a396..98a78d47 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TestUtils.kt @@ -1,8 +1,13 @@ package graphql.kickstart.tools +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender import com.fasterxml.jackson.databind.ObjectMapper import graphql.ExecutionInput import graphql.GraphQL +import org.slf4j.LoggerFactory private val mapper = ObjectMapper() @@ -37,3 +42,19 @@ fun assertNull(actual: T) { fun assertNotNull(actual: T) { assert(actual != null) } + +fun withWarnings(block: () -> T): Pair> { + // the tests run in the library's module, which doesn't read logback's + listOf("ch.qos.logback.classic", "ch.qos.logback.core").forEach { name -> + ModuleLayer.boot().findModule(name).ifPresent { SchemaParser::class.java.module.addReads(it) } + } + val logger = LoggerFactory.getLogger("graphql.kickstart.tools") as Logger + val appender = ListAppender().apply { start() } + logger.addAppender(appender) + try { + val result = block() + return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) }.map { it.formattedMessage } + } finally { + logger.detachAppender(appender) + } +} diff --git a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt index 87132bd6..d89194f6 100644 --- a/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/TypeExtensionTest.kt @@ -210,41 +210,45 @@ class TypeExtensionTest { @Test fun `directives of extensions follow those of the type and are wired`() { val wiring = RecordingWiring() - val schema = parser( - """ - directive @tag(name: String!) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT - directive @once(name: String!) on OBJECT | INPUT_OBJECT + val (schema, warnings) = withWarnings { + parser( + """ + directive @tag(name: String!) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT + directive @once(name: String!) on OBJECT | INPUT_OBJECT - type Query @once(name: "Query") { - node: Node - fooBar: FooBar - color: Color - search(filter: Filter): String - } + type Query @once(name: "Query") { + node: Node + fooBar: FooBar + color: Color + search(filter: Filter): String + } - extend type Query @once(name: "Query extension") + extend type Query @once(name: "Query extension") - interface Node @tag(name: "Node") { id: ID! } - extend interface Node @tag(name: "Node extension") + interface Node @tag(name: "Node") { id: ID! } + extend interface Node @tag(name: "Node extension") - type Item implements Node @tag(name: "Item") { id: ID! } - extend type Item @tag(name: "Item extension") { name: String } + type Item implements Node @tag(name: "Item") { id: ID! } + extend type Item @tag(name: "Item extension") { name: String } - union FooBar @tag(name: "FooBar") = Foo - extend union FooBar @tag(name: "FooBar extension") + union FooBar @tag(name: "FooBar") = Foo + extend union FooBar @tag(name: "FooBar extension") - enum Color @tag(name: "Color") { RED GREEN } - extend enum Color @tag(name: "Color extension") + enum Color @tag(name: "Color") { RED GREEN } + extend enum Color @tag(name: "Color extension") - input Filter @tag(name: "Filter") { id: ID } - extend input Filter @tag(name: "Filter extension") @once(name: "Filter extension") + input Filter @tag(name: "Filter") { id: ID } + extend input Filter @tag(name: "Filter extension") @once(name: "Filter extension") - type Foo { foo: String } - """) { - dictionary(Item::class, Foo::class) - directive("tag", wiring) - directive("once", wiring) - }.makeExecutableSchema() + type Foo { foo: String } + """) { + dictionary(Item::class, Foo::class) + directive("tag", wiring) + directive("once", wiring) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf("Directive @once isn't repeatable but an extension of type 'Query' applies it again, only the first one is kept.")) val applied = listOf("Query", "Node", "Item", "FooBar", "Color", "Filter").associateWith { name -> (schema.getType(name) as GraphQLDirectiveContainer).appliedDirectives.map { it.describe() } @@ -304,12 +308,15 @@ class TypeExtensionTest { $fooBarBaz """ - assertEquals(parser(unusedUnion).getUnusedDefinitions().map { it.javaClass.simpleName to it.name }.toSet(), setOf( + val (unused, warnings) = withWarnings { parser(unusedUnion).getUnusedDefinitions() } + + assertEquals(unused.map { it.javaClass.simpleName to it.name }.toSet(), setOf( "UnionTypeDefinition" to "FooBar", "ObjectTypeDefinition" to "Foo", "ObjectTypeDefinition" to "Bar", "ObjectTypeDefinition" to "Baz" )) + assertEquals(warnings, listOf("FooBar", "Foo", "Bar", "Baz").map { "Schema type was defined but can never be accessed, and can be safely deleted: $it" }) val schema = parser(unusedUnion) { dictionary(Foo::class, Bar::class, Baz::class) @@ -321,35 +328,43 @@ class TypeExtensionTest { @Test fun `repeats and extensions without a type keep building`() { - val schema = parser( - """ - type Query { - color: Color - node: Node - fooBar: FooBar - } + val (schema, warnings) = withWarnings { + parser( + """ + type Query { + color: Color + node: Node + fooBar: FooBar + } - extend enum Color { - "The first red" - RED - } - extend enum Color { RED GREEN } + extend enum Color { + "The first red" + RED + } + extend enum Color { RED GREEN } - interface Node { - "The base id" - id: ID! - } - extend interface Node { id: ID! name: String } + interface Node { + "The base id" + id: ID! + } + extend interface Node { id: ID! name: String } - type Item implements Node { id: ID! name: String } + type Item implements Node { id: ID! name: String } - extend union FooBar = Foo | Bar - extend union FooBar = Bar | Baz + extend union FooBar = Foo | Bar + extend union FooBar = Bar | Baz - $fooBarBaz - """) { - dictionary(Item::class, Foo::class, Bar::class, Baz::class) - }.makeExecutableSchema() + $fooBarBaz + """) { + dictionary(Item::class, Foo::class, Bar::class, Baz::class) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "Type 'Node' declares field 'id' more than once, only the first one is kept.", + "Type 'FooBar' declares member 'Bar' more than once, only the first one is kept.", + "Type 'Color' declares value 'RED' more than once, only the first one is kept." + )) val color = schema.getType("Color") as GraphQLEnumType assertEquals(color.values.map { it.name }, listOf("RED", "GREEN")) @@ -364,20 +379,28 @@ class TypeExtensionTest { @Test fun `extensions of a different kind than the type are ignored`() { - val schema = parser( - """ - type Query { node: Node } + val (schema, warnings) = withWarnings { + parser( + """ + type Query { node: Node } - interface Node { id: ID! } - extend type Node { name: String } + interface Node { id: ID! } + extend type Node { name: String } - type Item implements Node { id: ID! } - extend union Item = Foo + type Item implements Node { id: ID! } + extend union Item = Foo - type Foo { foo: String } - """) { - dictionary(Item::class) - }.makeExecutableSchema() + type Foo { foo: String } + """) { + dictionary(Item::class) + }.makeExecutableSchema() + } + + assertEquals(warnings, listOf( + "Type 'Node' is defined as an interface, but extended as an object type. The extension is ignored.", + "Type 'Item' is defined as an object type, but extended as a union. The extension is ignored.", + "Schema type was defined but can never be accessed, and can be safely deleted: Foo" + )) val node = schema.getType("Node") as GraphQLInterfaceType val item = schema.getType("Item") as GraphQLObjectType diff --git a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt index 1069304a..da8f5acd 100644 --- a/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt +++ b/src/test/kotlin/graphql/kickstart/tools/UnmappedInputFieldsTest.kt @@ -1,9 +1,5 @@ package graphql.kickstart.tools -import ch.qos.logback.classic.Level -import ch.qos.logback.classic.Logger -import ch.qos.logback.classic.spi.ILoggingEvent -import ch.qos.logback.core.read.ListAppender import com.fasterxml.jackson.annotation.JacksonInject import com.fasterxml.jackson.annotation.JsonCreator import com.fasterxml.jackson.annotation.JsonFormat @@ -32,7 +28,6 @@ import graphql.kickstart.tools.UnmappedInputFieldsFixtures.* import graphql.schema.DataFetchingEnvironment import org.junit.Assert.assertThrows import org.junit.Test -import org.slf4j.LoggerFactory import java.util.* class UnmappedInputFieldsTest { @@ -809,22 +804,6 @@ class UnmappedInputFieldsTest { "Input field '$field' has no matching property in ${clazz.name}. Converting $usage $consequence. Known properties: $knownProperties. " + "Rename the property or map it with @JsonProperty(\"${field.substringAfter('.')}\"). If it's left unmapped on purpose, list it in @JsonIgnoreProperties on the class." - private fun withWarnings(build: () -> T): Pair> { - // the tests run in the library's module, which doesn't read logback's - listOf("ch.qos.logback.classic", "ch.qos.logback.core").forEach { name -> - ModuleLayer.boot().findModule(name).ifPresent { javaClass.module.addReads(it) } - } - val logger = LoggerFactory.getLogger(org.slf4j.Logger.ROOT_LOGGER_NAME) as Logger - val appender = ListAppender().apply { start() } - logger.addAppender(appender) - try { - val result = build() - return result to appender.list.filter { it.level.isGreaterOrEqual(Level.WARN) }.map { it.formattedMessage } - } finally { - logger.detachAppender(appender) - } - } - class GreetingInput { var nmae: String? = null }