[READY] Initial defer implementation #553
Conversation
| @@ -126,11 +126,15 @@ abstract class NadelIntegrationTest( | |||
| // Maybe this won't hold out longer term, but e.g. it's ok for the deferred errors to add a path | |||
| assertJsonEquals( | |||
There was a problem hiding this comment.
This change is because we want to ignore the locations field in errors when comparing defer vs non-defer queries because the line/column of the error would be different as the two queries look slightly different (one has @defer directive stripped)
.../graphql/nadel/tests/next/fixtures/defer/ComprehensiveDeferQueryWithDifferentServiceCalls.kt
Outdated
Show resolved
Hide resolved
sbarker2
changed the title
| import graphql.nadel.NadelExecutionHints | ||
| import graphql.nadel.tests.next.NadelIntegrationTest | ||
|
|
||
| open class DeferThrowsErrorTest : NadelIntegrationTest( |
There was a problem hiding this comment.
What's the difference between this test and DeferReturnsErrorTest ?
There was a problem hiding this comment.
I think for DeferReturnsErrorTest I was meaning to return a DataFetcherResult with an error as that was the case that was failing before your change in graphql-java (graphql-java/graphql-java#3640).
I think I was supposed to do:
.dataFetcher("slow") { env ->
DataFetcherResult.newResult<String>()
.error(
GraphqlErrorBuilder.newError()
.message("An error occurred while fetching 'slow'")
.build()
)
.build()
}
I will delete DeferReturnsErrorTest for now and add in the new test later when we have updated graphql-java
test/src/test/kotlin/graphql/nadel/tests/next/fixtures/defer/DeferWithErrorTestSnapshot.kt
Outdated
Show resolved
Hide resolved
...est/kotlin/graphql/nadel/tests/next/fixtures/defer/MultipleDeferWithDifferentServiceCalls.kt
Outdated
Show resolved
Hide resolved
...in/graphql/nadel/tests/next/fixtures/defer/MultipleDeferWithDifferentServiceCallsSnapshot.kt
Outdated
Show resolved
Hide resolved
test/src/test/kotlin/graphql/nadel/tests/next/fixtures/defer/NestedDefersTest.kt
Outdated
Show resolved
Hide resolved
|
LGTM. Awesome test coverage Steven! |
Merging deferred results to publisher and send back to client (part of the defer implementation for non-hydrated queries)
NOTE: This does not apply the transformers yet - this work is in progress, but just note at this point, queries with transformers (for example renames) will not work.