Skip to content
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

Merged
merged 5 commits into from
Apr 17, 2024
Merged

Conversation

sbarker2
Copy link
Collaborator

@sbarker2 sbarker2 commented Mar 5, 2024

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.

@sbarker2 sbarker2 requested review from gnawf and felipe-gdr March 5, 2024 23:58
@@ -180,7 +180,11 @@ private suspend fun execute(

if (indexOfCall != null) {
val serviceCall = serviceCalls.removeAt(indexOfCall)
serviceCall.response
if (serviceCall.incrementalResponse != null) {
Copy link
Contributor

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

Copy link
Collaborator Author

@sbarker2 sbarker2 Mar 11, 2024

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 {
Copy link
Collaborator Author

@sbarker2 sbarker2 Mar 11, 2024

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
Copy link
Collaborator Author

@sbarker2 sbarker2 Mar 11, 2024

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")
Copy link
Contributor

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.

@sbarker2 sbarker2 merged commit 81f3384 into master Apr 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants