graphql-dotnet / graphql-client

A GraphQL Client for .NET Standard
MIT License
625 stars 134 forks source link

Testable Subscriptions with compatibility for TestServer #354

Open andreyleskov opened 3 years ago

andreyleskov commented 3 years ago

Hello everybody,

I'm using graphql-dotnet project a lot and would like to thank you for the amazing product you build for the whole .Net community. My company is building GraphQl-first services and relies a lot on integration testing with TestServer.

graphql-client Queries and Mutations work fine with TestServer because it is possible to pass an IHttpClient instance into GraphQlHttpClient constructor. But for subscription, there is no way to pass WebSocket from TestServer due to the not-configurable creation of ClientWebSocket at https://github.com/graphql-dotnet/graphql-client/blob/b366e22af112445452440c2c6c1d9978ecef97cb/src/GraphQL.Client/Websocket/GraphQLHttpWebSocket.cs#L403

My suggestion is to add a new constructor

    public GraphQLHttpWebSocket(Uri webSocketUri, GraphQLHttpClient client, Func<WebSocket> webSocketFactory)

to get better control over WebSocket creation. It would also allow moving condition compilation logic '#if NETFRAMEWORK' out of the GraphQLHttpWebSocket class later on, making it cleaner.

I see a lot of value in supporting subscriptions for TestServer. Our particular scenario is to issue asynchronous mutations and wait for some delayed effect via subscription.

What do you think about such a suggestion? If you find this idea worthy, I can prepare a PR for your review and further discussion.

rose-a commented 3 years ago

Hi,

I agree the current setup for integration-testing against a webserver setup is not elegant. Currently it's spinning up an actual webserver on a free port on the testserver.

I'd love to see an elegant solution to this. Please go ahead and prove your concept in the integration tests project.

andreyleskov commented 3 years ago

Hi rose-a,

I'm glad you like the idea, will prepare the PR for your review this week.

rose-a commented 3 years ago

I'm actually hoping for improved stability on the integration test through this, too (see #161). 😉

andreyleskov commented 3 years ago

I already spotted this issue locally, not sure about its nature yet. But it happens both for TestServer and the real server.

andreyleskov commented 3 years ago

Submitted https://github.com/graphql-dotnet/graphql-client/pull/357

rose-a commented 3 years ago

Thanks for your work, I'll try to review this over the weekend...

andreyleskov commented 3 years ago

@rose-a should I provide more explanations or details to help you out a bit?

rose-a commented 3 years ago

I'm really sorry, but I didn't get around to this yet... It's still on my To-Do list though.

andreyleskov commented 2 years ago

Any updates, @rose-a? One year gone )

pascal20997 commented 1 year ago

I'm facing the same issue. It is very inconvenient not be able to use the GraphQL client properly in the tests. Is there any news?

alexander-jesner-AP commented 10 months ago

Hi, any update on this one, @rose-a? Proper support for TestServer would be of great benefit for the community.

PKragelund commented 4 months ago

Any updates on this? 🙏

rose-a commented 4 months ago

No updates on this yet, as I am very limited on time myself. It would certainly help a lot if someone would update the corresponding MR to resolve the merge conflicts though.