MockServer improvements and TestNetworkTransport #3757

BoD · 2021-12-23T16:25:23Z

Step 1

0f83009
Add MockDispatcher interface which has a fun dispatch(request: MockRecordedRequest): MockResponse and modify MockServer to use it
Add an implementation QueueMockDispatcher, make it the default one in MockServer
Now MockServer.enqueue delegates to this queue dispatcher

Caveat: the Apple implementation of MockServer needs to freeze() the MockDispatcher, which prompted me to declare a copy method on the MockDispatcher interface - this allows to mutate the copy and re-freeze. Could there be a better way?

Step 2

28a676f
In apollo-testing-support, add ApolloMockDispatcher interface which is analogous to MockDispatcher but returning ApolloResponse
Thanks to a ApolloMockDispatcherBridge class and a fun MockServer(apolloMockDispatcher: ApolloMockDispatcher) function, it can be used as a regular MockDispatcher
Add 2 implementations:
- QueueApolloMockDispatcher : analogous to QueueMockDispatcher
- MapApolloMockDispatcher : has methods to associate Operation or Operation.Data to ApolloResponse

Step 3

3874543
In apollo-testing-support, add TestNetworkTransport and a TestNetworkTransportDispatcher interface
TestNetworkTransportDispatcher has 2 implementations:
- QueueTestNetworkTransportDispatcher
- MapTestNetworkTransportDispatcher (the default one)
This is basically what was described in TestNetworkTransport #3737, minus the “magic” to have the ability to pass a test builder lambda

Thoughts

After looking at this, I am not convinced we should keep what was done in Step 2 (ApolloMockDispatcher), because:

I find the TestNetworkTransport to be a better fitting (less low-level) API for users wishing to mock ApolloClient responses
For our own internal tests, there was already a MockServer.enqueue(Operation, Data) extension in apollo-testing-support which addresses pretty much the same usecase (I saw it later 🙈)
With the dispatcher approach you can’t both enqueue a string and an Operation on the same MockServer

…eueMockDispatcher). Note: Apple implementation works with a trick: avoiding a freeze() by using the new memory model (experimental).

martinbonnin · 2022-01-04T10:19:32Z

apollo-api/api/apollo-api.api

@@ -101,6 +101,12 @@ public final class com/apollographql/apollo3/api/ApolloResponse$Builder {
 	public final fun requestUuid (Ljava/util/UUID;)Lcom/apollographql/apollo3/api/ApolloResponse$Builder;
 }

+public final class com/apollographql/apollo3/api/ApolloResponsesKt {


Use @JvmName("ApolloResponses") to make Java interop nicer?

Also what about merging with ResponseParser and have something like ApolloResponseAdapter that can adapt to//from json like we do for other models?

apollo-mockserver/src/appleMain/kotlin/com/apollographql/apollo3/mockserver/MockServer.kt

apollo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServer.kt

...rver/src/commonTest/kotlin/com/apollographql/apollo3/mockserver/test/MockServerCommonTest.kt

…pollo's, which was breaking a few tests

…ver back. Freezing the MockDispatcher requires to implement a copy() method.

…lo3/mockserver/MockServer.kt Co-authored-by: Martin Bonnin <martin@mbonnin.net>

...ting-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/ApolloMockDispatcher.kt

settings.gradle.kts

...work-transport/src/test/kotlin/testnetworktransport/MapTestNetworkTransportDispatcherTest.kt

...lo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServerCommon.kt

...kserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/QueueMockServerHandler.kt

…ueueMockServerHandler for Apple instead

martinbonnin · 2022-01-05T09:39:13Z

...ting-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/TestNetworkTransport.kt

+  private val queue = ArrayDeque<ApolloResponse<out Operation.Data>>()
+
+  fun <D : Operation.Data> enqueue(response: ApolloResponse<D>) {
+    queue.add(response)


I think this needs to be locked. On native it doesn't throw because we run the flow on the main thread always but it might not always be the case. And on the JVM we cannot make any assumption about what dispatcher is going to be used.

martinbonnin

One last concurrency comment but looks great 🤩 . That'll be a nice addition!

BoD · 2022-01-05T17:44:07Z

Added some documentation here

…nstead have QueueTestNetworkTransport and MapTestNetworkTransport

BoD added 3 commits December 23, 2021 01:28

Add MockDispatcher. Now enqueue is a special case of dispatch (via Qu…

0f83009

…eueMockDispatcher). Note: Apple implementation works with a trick: avoiding a freeze() by using the new memory model (experimental).

Add ApolloMockDispatcher + Map and Queue implementations

28a676f

Add TestNetworkTransport + Map and Queue dispatchers

3fd55fb

BoD force-pushed the mockserver-improvements branch from 3874543 to 3fd55fb Compare December 23, 2021 16:40

Update apiDump

3c36f9b

martinbonnin reviewed Jan 4, 2022

View reviewed changes

apollo-mockserver/src/appleMain/kotlin/com/apollographql/apollo3/mockserver/MockServer.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

apollo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServer.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

apollo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServer.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...rver/src/commonTest/kotlin/com/apollographql/apollo3/mockserver/test/MockServerCommonTest.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...rver/src/commonTest/kotlin/com/apollographql/apollo3/mockserver/test/MockServerCommonTest.kt Outdated Show resolved Hide resolved

BoD and others added 4 commits January 4, 2022 14:43

Fix recorded requests kept too early in JS MockServer

9e22ad9

Fix body being consumed when converting OkHttp's RecordedRequest to A…

a1240c4

…pollo's, which was breaking a few tests

Remove experimental memory model, and put the freeze in Apple MockSer…

b318bd7

…ver back. Freezing the MockDispatcher requires to implement a copy() method.

Update apollo-mockserver/src/commonMain/kotlin/com/apollographql/apol…

9ed8c4e

…lo3/mockserver/MockServer.kt Co-authored-by: Martin Bonnin <martin@mbonnin.net>

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...ting-support/src/commonMain/kotlin/com/apollographql/apollo3/testing/ApolloMockDispatcher.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

settings.gradle.kts Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...work-transport/src/test/kotlin/testnetworktransport/MapTestNetworkTransportDispatcherTest.kt Outdated Show resolved Hide resolved

Reorganize tests

2d2717b

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...lo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServerCommon.kt Outdated Show resolved Hide resolved

BoD added 7 commits January 4, 2022 15:53

Rename MockDispatcher -> MockServerHandler

9dda05e

Rename MockRecordedRequest -> MockRequest

51336d5

Rename TestNetworkTransportDispatcher -> TestNetworkTransportHandler

7e7e0e4

A few more dispatcher -> handler renames

9667216

A few more dispatcher -> handler renames

7b9699d

Get rid of BaseMockServer which was not very useful

313ef8b

Remove the "manual" version of the TestNetworkTransport test

efb213e

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...lo-mockserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/MockServerCommon.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Jan 4, 2022

View reviewed changes

...kserver/src/commonMain/kotlin/com/apollographql/apollo3/mockserver/QueueMockServerHandler.kt Outdated Show resolved Hide resolved

Remove MockServerHandler.copy() and have a "freeze safe" version of Q…

f806cfa

…ueueMockServerHandler for Apple instead

BoD added 2 commits January 4, 2022 20:02

Remove ApolloMockServerHandler which was not really useful in the end

543f7b7

Catch exceptions in handler and respond 500 instead of crashing

760640f

martinbonnin reviewed Jan 5, 2022

View reviewed changes

martinbonnin approved these changes Jan 5, 2022

View reviewed changes

BoD added 3 commits January 5, 2022 14:46

Use TestNetworkTransport instead of MockWebServer in a few tests

e8621e8

Mark TestNetworkTransport as @ApolloExperimental

9b3b5fe

Add locks around collections in TestNetworkTransportHandlers

a32be34

martinbonnin approved these changes Jan 5, 2022

View reviewed changes

Add a custom TestNetworkTransportHandler test

b598cc9

Default to QueueTestNetworkTransportHandler in TestNetworkTransport

f85d0e6

BoD marked this pull request as ready for review January 10, 2022 10:14

BoD requested review from sav007 and tasomaniac as code owners January 10, 2022 10:14

Remove TestNetworkTransportHandler which was a bit over-engineered, i…

4caac2f

…nstead have QueueTestNetworkTransport and MapTestNetworkTransport

martinbonnin approved these changes Jan 10, 2022

View reviewed changes

BoD merged commit f476e02 into main Jan 10, 2022

BoD deleted the mockserver-improvements branch January 10, 2022 17:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MockServer improvements and TestNetworkTransport #3757

MockServer improvements and TestNetworkTransport #3757

BoD commented Dec 23, 2021 •

edited

Loading

martinbonnin Jan 4, 2022

martinbonnin Jan 4, 2022

martinbonnin Jan 5, 2022

martinbonnin left a comment

BoD commented Jan 5, 2022

MockServer improvements and TestNetworkTransport #3757

MockServer improvements and TestNetworkTransport #3757

Conversation

BoD commented Dec 23, 2021 • edited Loading

Step 1

Step 2

Step 3

Thoughts

martinbonnin Jan 4, 2022

Choose a reason for hiding this comment

martinbonnin Jan 4, 2022

Choose a reason for hiding this comment

martinbonnin Jan 5, 2022

Choose a reason for hiding this comment

martinbonnin left a comment

Choose a reason for hiding this comment

BoD commented Jan 5, 2022

BoD commented Dec 23, 2021 •

edited

Loading