-
Notifications
You must be signed in to change notification settings - Fork 24
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
add defer types to test fixture #520
Conversation
@@ -180,7 +180,11 @@ private suspend fun execute( | |||
|
|||
if (indexOfCall != null) { | |||
val serviceCall = serviceCalls.removeAt(indexOfCall) | |||
serviceCall.response | |||
if (serviceCall.incrementalResponse != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's fail the test when both response
and incrementalResponse
are defined in the fixture? It never makes sense to define both together
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call 👍
@@ -168,6 +168,18 @@ private suspend fun execute( | |||
) | |||
printSyncLine(actualQuery) | |||
|
|||
fun failWithFixtureContext(message: String): Nothing { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved this into function because the fixture context is now useful for multiple fail messages
@@ -180,17 +192,20 @@ private suspend fun execute( | |||
|
|||
if (indexOfCall != null) { | |||
val serviceCall = serviceCalls.removeAt(indexOfCall) | |||
serviceCall.response | |||
val response = serviceCall.response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assigning serviceCall.response
to a variable, because kotlin thinks the state can change due to the way it was defined:
val response: JsonMap? by lazy {
...
}
@@ -88,3 +94,20 @@ data class ExpectedException( | |||
) | |||
} | |||
} | |||
|
|||
data class IncrementalResponse( | |||
@JsonProperty("initialResponse") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to merge this as it is. But as we discussed, it's worth trying to use the actual graphql-java types here to make deserialization easier.
Note: the tests arent asserting the incremental responses yet for deferred requests, it is just using the initial responses at the moment, the work for the incremental responses is in progress and will be in a follow-up PR.
The aim of this PR is just to add all the types of the test fixture.