graphql-client
graphql-client copied to clipboard
Add TestServer support for GraphQLHttpClient #354
This PR makes GraphQLHttpClient usable with TestServer, targeting #354
Usage
TestServer testServer; //needs initialization
GraphQLHttpClientOptions options; //needs initialization
IGraphQLWebsocketJsonSerializer serializer; //needs initialization
var graphQLHttpClient = testServer.CreateGraphQLHttpClient(options,serializer);
Implementation GraphQLHttpClient and GraphQLHttpWebSocket have a new internal constructor with a new factory parameter for connected WebSocket creation.
Func<Uri, CancellationToken,Task<WebSocket>> connectedWebSocketFactory
This parameter is used to provide TestWebSocket instance from TestServer. There is a new project GraphQL.Client.TestHost with the extensions method to configure this factory for a given TestServer.
Discussion
- Need to check if net461 build is ok, as I checked it only with Mono under MacOS, would be nice to check with full .Net.
- There is
CanHandleConnectionTimeout
integration test that I could not make work for TestServer. Seems like TestServer does not drop WebSocket connection on dispose. I could be wrong. Any help or suggestions are welcome. As a shortcut, I moved this test out of scope for TestServer - Way to inject WebSocket into GraphQLHttpClient. Now it is dictated by TestServer's WebSocketClient, and, maybe, there is a better way other than GraphQLHttpClient internal constructor requiring complex factory setup and separate project usage.
- Tests stability, as mentioned in #161. I can see the same behavior for TestServer and think it is caused by synchronization inside tests themselves working with the same server instance. This issue is not targeted by the PR, but it makes this hypothesis check easier a bit by allowing to run a dedicated TestServer for each test.
~~@andreyleskov please rebase your branch on release-v4
~~
already done
Hey guys, can I do smth to make this PR move? Please let me know
Hello, what about this PR. I would like to use this... @rose-a ????
Hi, @rose-a, is it possible to merge this PR? If not, how can we help to push it towards being mergeable?
This would have been a lifesaver...