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

HubConnection interface #14924

Closed
turowicz opened this issue Oct 11, 2019 · 7 comments
Closed

HubConnection interface #14924

turowicz opened this issue Oct 11, 2019 · 7 comments
Labels
area-signalr Includes: SignalR clients and servers Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.

Comments

@turowicz
Copy link

turowicz commented Oct 11, 2019

In contrast to the "full" SignalR, Core one doesn't have a HubConnection that would be possible to Mock. There is no interface for it.

https://github.com/aspnet/AspNetCore/blob/master/src/SignalR/clients/csharp/Client.Core/src/HubConnection.cs#L36

@turowicz
Copy link
Author

Having similar problems with mocking WithUrl method on the IHubConnectionBuilder. Moq doesn't support extension methods.

It's as if nobody considered unit testing when designing contracts for Signalr.

@mkArtakMSFT mkArtakMSFT added the area-signalr Includes: SignalR clients and servers label Oct 11, 2019
@halter73
Copy link
Member

We should definitely consider making more HubConnection methods virtual.

As for WithUrl, that is already mockable. It's true you cannot override the static extension method yourself, but you can the configured URL as follows after calling WithUrl:

HubConnectionBuilder.WithUrl("https://example.com/");
var configuredUri = HubConnectionBuilder.Services.BuildServiceProvider()
                        .GetService<IOptions<HttpConnectionOptions>>().Value.Url;
Assert.Equal(new Uri("https://example.com/"), configuredUri);

This should work either with a real HubConnectionBuilder or any mocked IHubConnectionBuilder with a realistic IServiceCollection implementation. Even if you you wind up providing a custom IHubConnectionBuilder.Build() implementation, I would recommend just using the default IServiceCollection implementation which is really just a thin wrapper around List<ServiceDescriptor>.

@turowicz
Copy link
Author

@halter73 I think you should put it behind an interface and don't base your code architecture on extension methods as they are pain to work with.

We build mission-critical systems that people's lives depend on. We simply cannot work with a technology that is not unit testable.

@davidfowl
Copy link
Member

@turowicz you can always wrap the hub connection in your own interface.

@analogrelay
Copy link
Contributor

Can you provide an example of the test you want to perform? Understanding what kind of mocking you want here would be helpful. Adding interfaces makes the framework code itself much more brittle to breaking changes over time, so we don't just introduce them for testability purposes (when it's always feasible to write a wrapper if you need to do this kind of unit testing).

@analogrelay analogrelay added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Oct 15, 2019
@analogrelay
Copy link
Contributor

Closing this as we haven't heard from you and generally close issues with no response after some time. Please feel free to comment if you're able to get the information we're looking for and we can reopen the issue to investigate further!

@turowicz
Copy link
Author

@anurse @davidfowl I will follow your best practice of wrapping everything into interfaces myself.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-signalr Includes: SignalR clients and servers Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue.
Projects
None yet
Development

No branches or pull requests

5 participants