graphql-client icon indicating copy to clipboard operation
graphql-client copied to clipboard

Add TestServer support for GraphQLHttpClient #354

Open andreyleskov opened this issue 3 years ago • 5 comments

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

  1. 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.
  2. 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
  3. 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.
  4. 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 avatar Jun 29 '21 10:06 andreyleskov

~~@andreyleskov please rebase your branch on release-v4~~

already done

rose-a avatar Sep 14 '21 08:09 rose-a

Hey guys, can I do smth to make this PR move? Please let me know

andreyleskov avatar Feb 20 '22 10:02 andreyleskov

Hello, what about this PR. I would like to use this... @rose-a ????

jerry2007 avatar Mar 15 '23 13:03 jerry2007

Hi, @rose-a, is it possible to merge this PR? If not, how can we help to push it towards being mergeable?

alexander-jesner-AP avatar Jan 26 '24 11:01 alexander-jesner-AP

This would have been a lifesaver...

tomachristian avatar Mar 22 '24 16:03 tomachristian