Runtime: An simple way to mock an httpClient.GetAsync(..) method for unit tests?

[PureKrome]

System.Net.Http has now been uploaded to the repo :smile: :tada: :balloon:

Whenever I've used this in some service, it works great but makes it hard to unit test => my unit tests don't want to actually ever hit that real end point.

Ages ago, I asked @davidfowl what should we do? I hoping I paraphrase and don't misquote him - but he suggested that I need to fake up a message handler (ie. HttpClientHandler), wire that up, etc.

As such, I ended up making a helper library called HttpClient.Helpers to help me run my unit tests.

So this works ... but it feels _very_ messy and .. complicated. I'm sure I'm not the first person that needs to make sure my unit tests don't do a real call to an external service.

Is there an easier way? Like .. can't we just have an IHttpClient interface and we can inject that into our service?

[sharwell]

HttpClient is nothing more than an abstraction layer over another HTTP library. One of its primary purposes is allowing you to replace the behavior (implementation), including but not limited to the ability to create deterministic unit tests.

In other works, HttpClient itself serves as the both the "real" and the "mock" object, and the HttpMessageHandler is what you select to meet the needs of your code.

[PureKrome]

But if feels like sooo much work is required to setup the message handler when (it feels like) this could be handled with a nice interface? Am I totally misunderstanding the solution?

[SidharthNabar]

@PureKrome - thanks for bringing this up for discussion. Can you please elaborate on what you mean by "so much work is required to setup the message handler"?

One way to unit test HttpClient without hitting the network is -

1. Create a new Handler class (e.g. FooHandler) that derives from HttpMessageHandler
2. Implement the SendAsync method as per your requirements - don't hit the network/log the request-response/etc.
3. Pass an instance of this FooHandler into the constructor of the HttpClient:
   var handler = new FooHandler();
   var client = new HttpClient(handler);

Your HttpClient object will then use your Handler instead of the inbuilt HttpClientHandler.

Thanks,
Sid

[PureKrome]

Hi @SidharthNabar - Thank you for reading my issue, I really appreciate it also.

Create a new Handler class

You just answered my question :)

That's a large dance to wiggle too, just to ask my real code to not _hit the network_.

I even made a the HttpClient.Helpers repo and nuget package .. just to make testing easier! Scenario's like the Happy path or an exception thrown by the network endpoint ...

That's the problem -> can we not do all of this and ... just a mock a method instead?

I'll try and explain via code..

Goal: Download something from the interwebs.

public async Foo GetSomethingFromTheInternetAsync()
{
    ....
    using (var httpClient = new HttpClient())
    {
        html = await httpClient.GetStringAsync("http://www.google.com.au");
    }
    ....
}

Lets look at two examples:

Given an interface (if it existed):

public interface IHttpClient
{
    Task<string> GetStringAsync(string requestUri);    
}

My code can now look like this...

public class SomeService(IHttpClient httpClient = null)
{
    public async Foo GetSomethingFromTheInternetAsync()
    {
        ....
        using (var httpClient = _httpClient ?? new HttpClient()) // <-- CHANGE dotnet/corefx#1
        {
            html = await httpClient.GetStringAsync("http://www.google.com.au");
        }
        ....
    } 
}

and the test class

public async Task GivenAValidEndPoint_GetSomethingFromTheInternetAsync_ReturnsSomeData()
{
    // Create the Mock : FakeItEasy > Moq.
    var httpClient = A.Fake<IHttpClient>();

    // Define what the mock returns.
    A.CallTo(()=>httpClient.GetStringAsync("http://www.google.com.au")).Returns("some html here");

    // Inject the mock.
    var service = new SomeService(httpClient);
    ...
}

Yay! done.

Ok, now with the current way...

1. create a new Handler class - yes, a class!
2. Inject the handler into the service

public class SomeService(IHttpClient httpClient = null)
{
    public async Foo GetSomethingFromTheInternetAsync()
    {
        ....
        using (var httpClient = _handler == null 
                                    ? new HttpClient()
                                    : new HttpClient(handler))
        {
            html = await httpClient.GetStringAsync("http://www.google.com.au");
        }
        ....
    } 
}

But the pain is now that I have to make a class. You've now hurt me.

public class FakeHttpMessageHandler : HttpClientHandler
{
 ...
}

And this class starts out pretty simple. Until I when I have multiple GetAsync calls (so i need to provide multiple Handler instances?) -or- multiple httpClients in a single service. Also, do we Dispose of the handler or reuse it if we're doing multiple calls in the same block of logic (which is a ctor option)?

eg.

public async Foo GetSomethingFromTheInternetAsync()
{
    string[] results;
    using (var httpClient = new HttpClient())
    {
        var task1 = httpClient.GetStringAsync("http://www.google.com.au");
        var task2 = httpClient.GetStringAsync("http://www.microsoft.com.au");

        results = Task.WhenAll(task1, task2).Result;
    }
    ....
}

this can be made so much easier with this..

var httpClient = A.Fake<IHttpClient>();
A.CallTo(() = >httpClient.GetStringAsync("http://www.google.com.au"))
    .Returns("gooz was here");
A.CallTo(() = >httpClient.GetStringAsync("http://www.microsoft.com.au"))
    .Returns("ms was here");

clean clean clean :)

Then - there is the next bit : Discoverability

When I first started using MS.Net.Http.HttpClient the api was pretty obvious :+1: Ok - get string and do it asyncly.. simple ...

... but then I hit testing .... and now I'm suppose to learn about HttpClientHandlers? Um why? I feel that this should all be hidden under the covers and I shouldn't have to worry about all this implementation detail. It's too much! You're saying that I should start to look inside the box and learn some of the plumbing ... which hurts :cry:

It's all making this more complex than it should be, IMO.

Help me Microsoft - you're my only hope.

[aarondandy]

I too would love to see an easy and simple way to test various things that use HttpClient. :+1:

[SidharthNabar]

Thanks for the detailed response and code snippets - that really helps.

First, I notice that you are creating new instances of HttpClient for sending each request - that is not the intended design pattern for HttpClient. Creating one HttpClient instance and reusing it for all your requests helps optimize connection pooling and memory management. Please consider reusing a single instance of HttpClient. Once you do this, you can insert the fake handler into just one instance of HttpClient and you're done.

Second - you are right. The API design of HttpClient does not lend itself to a pure interface-based mechanism for testing. We are considering adding a static method/factory pattern in the future that will allow you to alter the behavior of all HttpClient instances created from that factory or after that method. We haven't yet settled on a design for this yet, though. But the key issue will remain - you will need to define a fake Handler and insert it below the HttpClient object.

@ericstj - thoughts on this?

Thanks,
Sid.

[PureKrome]

@SidharthNabar why are you/team so hesitant to offering an interface for this class? I was hoping that the whole point of a developer having to _learn_ about handlers and then having to _create_ fake handler classes is enough of a pain point to justify (or at the very least, highlight) the need for an interface?

[luisrudge]

Yeah, I don't see why an Interface would be a bad thing. It would make testing HttpClient so much easier.

[ericstj]

One of the main reasons we avoid interfaces in the framework is that they don't version well. Folks can always create their own if they have a need and that won't get broken when the next version of the framework needs to add a new member. @KrzysztofCwalina or @terrajobst might be able to share more of the history/detail of this design guideline. This particular issue is a near religious debate: I'm too pragmatic to take part in that.

HttpClient has lots of options for unit testability. It's not sealed and most of its members are virtual. It has a base class that can be used to simplify the abstraction as well as a carefully designed extension point in HttpMessageHandler as @sharwell points out. IMO it's a quite well designed API, thanks to @HenrikFrystykNielsen.

[TerribleDev]

:+1: an interface

[PureKrome]

:wave: @ericstj Thanks heaps for jumping in :+1: :cake:

One of the main reasons we avoid interfaces in the framework is that they don't version well.

Yep - great point.

This particular issue is a near religious debate

yeah ... point well taken on that.

HttpClient has lots of options for unit testability

Oh? I'm struggling on this :blush: hence the reason for this issue :blush:

It's not sealed and most of its members are virtual.

Members are virtual? Oh drats, I totally missed that! If they are virtual, then mocking frameworks can mock those members out :+1: and we don't need an interface!

Lets have a looksies at HttpClient.cs ...

public Task<HttpResponseMessage> GetAsync(string requestUri) { .. }
public Task<HttpResponseMessage> GetAsync(Uri requestUri) { .. }

hmm. those aren't virtual ... lets search the file ... er .. no virtual keyword found. So do you mean _other_ members to _other related_ classes are virtual? If so .. then we're back at the issue I'm raising - we have to now look under the hood to see what GetAsync is doing so we then know what to create/wireup/etc...

I guess I'm just not understanding something really basic, here ¯(°_°)/¯ ?

EDIT: maybe those methods can be virtual? I can PR!

[ericstj]

SendAsync is virtual and almost every other API layers on top of that. 'Most' was incorrect, my memory serves me wrong there. My impression was that most were effectively virtual since they build on top of a virtual member. Usually we don't make things virtual if they are cascading overloads. There is a more specific overload of SendAsync that is not virtual, that one could be fixed.

[PureKrome]

Ah! Gotcha :blush: So all those methods end up calling SendAsync .. which does all the heavy lifting. So this still means we have a discoverability issue here .. but lets put that aside (that's opinionated).

How would we mock SendAsync give this basic sample...
Install-Package Microsoft.Net.Http
Install-Package xUnit

public class SomeService
{
    public async Task<string> GetSomeData(string url)
    {
        string result;
        using (var httpClient = new HttpClient())
        {
            result = await httpClient.GetStringAsync(url);
        }

        return result;
    }
}

public class Facts
{
    [Fact]
    public async Task GivenAValidUrl_GetSomeData_ReturnsSomeData()
    {
        // Arrange.
        var service = new SomeService();

        // Act.
        var result = await service.GetSomeData("http://www.google.com");

        // Assert.
        Assert.True(result.Length > 0);
    }
}

[MrJul]

One of the main reasons we avoid interfaces in the framework is that they don't version well. Folks can always create their own if they have a need and that won't get broken when the next version of the framework needs to add a new member.

I can totally understand this way of thinking with the full .NET Framework.

However, .NET Core is split into many small packages, each versioned independently. Use semantic versioning, bump the major version when there's a breaking change, and you're done. The only people affected will be the ones explicitly updating their packages to the new major version - knowing there are documented breaking changes.

I'm not advocating that you should break every interface every day: breaking changes should ideally be batched together into a new major version.

I find it sad to cripple APIs for future compatibility reasons. Wasn't one of the goal of .NET Core to iterate faster since you don't have to worry anymore about a subtle .NET update breaking thousands of already installed applications?

My two cents.

[luisrudge]

@MrJul :+1:
Versioning shouldn't be a problem. That's why the version numbers exist :)

[ericstj]

Versioning is still very much an issue. Adding a member to an interface is a breaking change. For core libraries like this that are inbox in desktop we want to bring back features to desktop in future versions. If we fork it means folks can't write portable code that will run in both places. For more information on breaking changes have a look at: https://github.com/dotnet/corefx/wiki/Breaking-Changes.

I think this is a good discussion, but as I mentioned before I don't have a lot of scratch in the argument around interfaces vs abstract classes. Perhaps this is a topic to bring to the next design meeting?

I'm not super familiar with the test library you're using, but what I'd do is provide a hook to allow tests to set the instance of the client and then create a mock object that behaved however I want. The mock object could be mutable to permit some reuse.

If you have a specific compatible change you'd like to suggest to HttpClient that improves unit testability please propose it and @SidharthNabar and his team can consider it.

[KrzysztofCwalina]

If the API is not mockable, we should fix it. But it has nothing to do with interfaces vs classes. Interface is no different than pure abstract class from mockability perspective, for all practical purposes.

[luisrudge]

@KrzysztofCwalina you, sir, hit the nail on the head! perfect summary!

[drub0y]

I guess I'm going to take the less popular side of this argument as I personally do not think an interface is necessary. As has already been pointed out, HttpClient is the abstraction already. The behavior really comes from the HttpMessageHandler. Yes, it's not mockable/fakeable using the approaches we've become accustom to with frameworks like Moq or FakeItEasy, but it doesn't need to be; that's not the _only_ way there is do things. The API is still perfectly testable by design though.

So let's address the "I need to create my own HttpMessageHandler?". No, of course not. We didn't all write our own mocking libraries. @PureKrome has already shown his HttpClient.Helpers library. Personally I have not used that one, but I will check it out. I've been using @richardszalay's MockHttp library which I find fantastic. If you've ever worked with AngularJS's $httpBackend, MockHttp is taking exactly the same approach.

Dependency wise, your service class should allow an HttpClient to be injected and obviously can supply the reasonable default of new HttpClient(). If you need to be able to create instances, take a Func<HttpClient> or, if you're not a fan of Func<> for whatever reason, design your own IHttpClientFactory abstraction and a default implementation of that would, again, just return new HttpClient().

In closing, I find this API perfectly testable in its current form and see no need for an interface. Yes, it requires a different approach, but the aforementioned libraries already exist to help us with this differnt style of testing in perfectly logical, intuitive ways. If we want more functionality/simplicity let's contribute to those libraries to make them better.

[luisrudge]

Personally, an Interface or virtual methods doesn't matter that much to me. But c'mon, perfectly testable is a little to much :)

[drub0y]

@luisrudge Well can you give a scenario that can't be tested using the message handler style of testing that something like MockHttp enables? Maybe that would help make the case.

I haven't come across anything I couldn't codify yet, but maybe I'm missing some more esoteric scenarios that can't be covered. Even then, might just be a missing feature of that library that someone could contribute.

For now I maintain the opinion that it's "perfectly testable" as a dependency, just not in a way .NET devs are used to.

[luisrudge]

It's testable, I agree. but it's still a pain. If you have to use an external lib that helps you do that, it's not perfectly testable. But I guess that's too subjective to have a discussion over it.

[luisrudge]

One could argue that aspnet mvc 5 is perfectly testable, you just have to write 50LOC to mock every single thing a controller needs. IMHO, that's the same case with HttpClient. It's testable? Yes. It's easy to test? No.

[drub0y]

@luisrudge Yes, I agree, it's subjective and I completely understand the desire for interface/virtuals. I'm just trying to make sure anyone who comes along and reads this thread will at least get some education on the fact that this API _can be_ leveraged in a code base in a very testable way without introducing all of your own abstractions around HttpClient to get there.

if you have to use an external lib that helps you do that, it's not perfectly testable

Well, we're all using one library or another for mocking/faking already^* and, it's true, we can't just use _that_ one for testing this style of API, but I don't think that means its any less testable than an interface based approach just because I have to bring in another library.

_^* At least I hope not!_

[PureKrome]

@drub0y from my OP I've stated that the library is testable - but it's just a real pain compared (to what i passionately believe) to what it _could_ be. IMO @luisrudge spelt it out perfectly :

One could argue that aspnet mvc 5 is perfectly testable, you just have to write 50LOC to mock every single thing a controller needs

This repo is a major part of a _large_ number of developers. So the default (and understandable) tactic is to be _very_ cautious with this repo. Sure - I totally get that.

I'd like to believe that this repo is still in a position to be tweaked (with respect to the API) before it goes RTW. Future changes suddenly become _really_ hard to do, include the _perception_ to change.

So with the current api - can it be tested? Yep. Does it pass the Dark Matter Developer test? I personally don't think so.

The litmus test IMO is this: can a regular joe pickup one of the common/popular test frameworks + mocking frameworks and mock any of the main methods in this API? Right now - nope. So then the developer needs to stop what they're doing and start learning about the _implimentation_ of this library.

As has already been pointed out, HttpClient is the abstraction already. The behavior really comes from the HttpMessageHandler.

This is my point - why are you making us having to spend cycles figuring this out when the library's purpose is to abstract all that magic .. only to say "Ok .. so we've done some magic but now that you want to mock our magic ... I'm sorry, you're now going to have to pull up your sleeves, lift up the roof of the library and start digging inside, etc".

It feels so ... convoluted.

We're in a position to make life easy for so many people and to keep coming back to the defensive position of : "It can be done - but .. read the FAQ/Code".

Here's another angle to approach this issue: Give 2 example to random Non-MS devs ... joe citizens developers that know how to use xUnit and Moq/FakeItEasy/InsertTheHipMockingFrameworkThisYear.

• First API that uses an interface/virtual/whatever .. but the method can be mocked.
• Second API that is just a public method and to mock the _integration point_ .. they have to read the code.

It distills down to that, IMO.

See which developer can solve that problem _and_ stay happy.

If the API is not mockable, we should fix it.

Right now it's not IMO - but there's ways to get around this successfully (again, that's opinionated - I'll concede that)

But it has nothing to do with interfaces vs classes. Interface is no different than pure abstract class from mockability perspective, for all practical purposes.

Exactly - that's an implementation detail. Goal is to be able to use a battle-tested mock framework and off you go.

@luisrudge if you have to use an external lib that helps you do that, it's not perfectly testable
@drub0y Well, we're all using one library or another for mocking/faking already

(I hope I understood that last quote/paragraph..) Not ... quiet. What @luisrudge was saying is: "We have one tool for testing. A second tool for mocking. So far, those a generic/overall tools not tied to anything. But .. you now want me to download a 3rd tool which is _specific_ to mocking a _specific_ API/service in our code because that specific API/service we're using, wasn't designed to be tested nicely?". That's a bit rich :(

Dependency wise, your service class should allow an HttpClient to be injected and obviously can supply the reasonable default of new HttpClient()

Completely agreed! So, can you refactor the sample code above to show us how easy this should/can be? There's many ways to skin a cat ... so what's a simple AND easy way? Show us the code.

I'll start. Appologies to @KrzysztofCwalina for using an interface, but this is just to kickstart my litmus test.

public interface IHttpClient
{
    Task<string> GetStringAsync(string url);
}

public class SomeService
{
    private IHttpClient _httpClient;

    // Injected.
    public SomeService(IHttpClient httpClient) { .. }

    public async Task<string> GetSomeData(string url)
    {
        string result;
        using (var httpClient = _httpClient ?? new HttpClient())
        {
            result = await httpClient.GetStringAsync(url);
        }

        return result;
    }    
}

[sharwell]

It sounds like all @PureKrome needs is a documentation update explaining which methods to mock/override in order to customize the behavior of the API during testing.

[luisrudge]

@sharwell that's absolutely not the case. When I test stuff, I don't want to run the entire httpclient code:
Look the SendAsync method.
It's insane. What you're suggesting is that every developer should know the internals from the class, open github, see the code and that kind of stuff to be able to test it. I want that developer to mock the GetStringAsync and get shit done.

[sharwell]

@luisrudge Actually my suggestion would be to avoid mocking for this library altogether. Since you already have a clean abstraction layer with a decoupled, replaceable implementation, additional mocking is unnecessary. Mocking for this library has the following additional downsides:

1. Increased burden on developers creating tests (maintaining the mocks), which in turn increases development costs
2. Increased likelihood that your tests will fail to detect certain problematic behaviors, such as reusing the content stream associated with a message (sending a message results in a call to Dispose on the content stream, but most mocks will ignore this)
3. Unnecessarily tighter coupling of the application to a particular abstraction layer, which increases development costs associated with evaluating alternatives to HttpClient

Mocking is a testing strategy targeting intertwined codebases that are difficult to test at a small scale. While the use of mocking correlates with increased development costs, the _need_ for mocking correlates with increased amount of coupling in the code. This means mocking itself is a code smell. If you can provide the same input-output test coverage across and within your API without using mocking, you will benefit in basically every development aspect.

[ctolkien]

Since you already have a clean abstraction layer with a decoupled, replaceable implementation, additional mocking is unnecessary

With respect, if the option is having to write your own Faked Message Handler, which isn't obvious without digging through the code, then that is a very high barrier that is going to be hugely frustrating for a lot of developers.

I agree with @luisrudge 's comment earlier. _Technically_ MVC5 was testable, but my god was it a pain in the ass and an immense source of hair pulling.

[luisrudge]

@sharwell Are you saying that mocking IHttpClient is a burden and implementing a fake message handler (like this one isn't? I can't agree with that, sorry.

[sharwell]

@luisrudge For simple cases of testing GetStringAsync, it's not nearly that hard to mock the handler. Using out-of-the-box Moq + HttpClient, all you need is this:

Uri requestUri = new Uri("http://google.com");
string expectedResponse = "Response text";

Mock<HttpClientHandler> mockHandler = new Mock<HttpClientHandler>();
mockHandler.Protected()
    .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.Is<HttpRequestMessage>(message => message.RequestUri == requestUri), ItExpr.IsAny<CancellationToken>())
    .Returns(Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(expectedResponse) }));
HttpClient httpClient = new HttpClient(mockHandler.Object);
string result = await httpClient.GetStringAsync(requestUri).ConfigureAwait(false);
Assert.AreEqual(expectedResponse, result);

If that's too much, you can define a helper class:

internal static class MockHttpClientHandlerExtensions
{
    public static void SetupGetStringAsync(this Mock<HttpClientHandler> mockHandler, Uri requestUri, string response)
    {
        mockHandler.Protected()
            .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.Is<HttpRequestMessage>(message => message.RequestUri == requestUri), ItExpr.IsAny<CancellationToken>())
            .Returns(Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(response) }));
    }
}

Using the helper class all you need is this:

Uri requestUri = new Uri("http://google.com");
string expectedResponse = "Response text";

Mock<HttpClientHandler> mockHandler = new Mock<HttpClientHandler>();
mockHandler.SetupGetStringAsync(requestUri, expectedResponse);
HttpClient httpClient = new HttpClient(mockHandler.Object);
string result = await httpClient.GetStringAsync(requestUri).ConfigureAwait(false);
Assert.AreEqual(expectedResponse, result);

[PureKrome]

So to compare the two versions:
_Disclaimer_: this is untested, browser code. Also, I haven't used Moq in ages.

Current

public class SomeService(HttpClientHandler httpClientHandler= null)
{
    public async Foo GetSomethingFromTheInternetAsync()
    {
        ....
        using (var httpClient = _handler == null
                                  ? new HttpClient
                                  : new HttpClient(handler))
        {
            html = await httpClient.GetStringAsync("http://www.google.com.au");
        }
        ....
    } 
}

// Unit test...
// Arrange.
Uri requestUri = new Uri("http://google.com");
string expectedResponse = "Response text";
var responseMessage = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(expectedResponse) };
Mock<HttpClientHandler> mockHandler = new Mock<HttpClientHandler>();
mockHandler.Protected()
    .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.Is<HttpRequestMessage>(message => message.RequestUri == requestUri), ItExpr.IsAny<CancellationToken>())
    .ReturnsAsync(responseMessage);
HttpClient httpClient = new HttpClient(mockHandler.Object);
var someService = new SomeService(mockHandler.Object); // Injected...

// Act.
string result = await someService.GetSomethingFromTheInternetAsync();

// Assert.
Assert.AreEqual(expectedResponse, result);
httpClient.Verify(x => x.GetSomethingFromTheInternetAsync(), Times.Once);

versus offering a way to mock the API method directly.

Another way

public class SomeService(IHttpClient httpClient = null)
{
    public async Foo GetSomethingFromTheInternetAsync()
    {
        ....
        using (var httpClient = _httpClient ?? new HttpClient())
        {
            html = await httpClient.GetStringAsync("http://www.google.com.au");
        }
        ....
    } 
}

// Unit tests...
// Arrange.
var response = new Foo();
var httpClient = new Mock<IHttpClient>();
httpClient.Setup(x => x.GetStringAsync(It.IsAny<string>))
    .ReturnsAsync(response);
var service = new SomeService(httpClient.Object); // Injected.

// Act.
var result = await service.GetSomethingFromTheInternetAsync();

// Assert.
Assert.Equals("something", foo.Something);
httpClient.Verify(x => x.GetStringAsync(It.IsAny<string>()), Times.Once);

distilling that down, it mainly comes to this (excluding the slight differece in SomeService) ...

var responseMessage = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(expectedResponse) };
Mock<HttpClientHandler> mockHandler = new Mock<HttpClientHandler>();
mockHandler.Protected()
    .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.Is<HttpRequestMessage>(message => message.RequestUri == requestUri), ItExpr.IsAny<CancellationToken>())
    .ReturnsAsync(responseMessage);
HttpClient httpClient = new HttpClient(mockHandler.Object);

vs this ...

var httpClient = new Mock<IHttpClient>();
httpClient.Setup(x => x.GetStringAsync(It.IsAny<string>))
    .ReturnsAsync(response);

Does the code _roughly_ look about right for both? (both have to be fairly accurate, before we can compare the two).

[luisrudge]

@sharwell so I have to run my code through all HttpClient.cs code in order to run my tests? How is that less coupled with HttpClient?

[raymens]

We heavily use HttpClient in some of our systems and I wouldn't call it a perfectly fine testable piece of tech. I really like the library but having some sort of Interface or any other improvement for testing would be a major improvement IMO.

Kudo's to @PureKrome for his library, but it would be better if it wasn't necessary :)

[tucaz]

I just read all comments and learned a lot, but I have one question: why do you need to mock the HttpClient in your tests?

In most common scenarios functionality provided by HttpClient will already be wrapped in some class such as HtmlDownloader.DownloadFile(). Why don't just mock this class instead of its plumbing?

Another possible scenario that would require such mocking is when you have to parse a downloaded string and this piece of code requires testing for multiple different outputs. Even in this case, this functionality (parsing) could be extracted away to a class/method and tested without any mocks.

I'm not saying that the testability of it cannot be improved, neither implying that it shouldn't be. I just want to understand to learn some new stuff.

[PureKrome]

@tucaz Lets imagine I have a website that returns some information about yourself. Your age, name, birthday, etc. You also happen to have stocks in some companies.

You've said you own 100 units of AAA stock and 200 units of BBB stock? So, when we display your account details we _should_ display how much money your stocks are worth.

To do this, we need to grab the current stock prices from _another_ system. So to do this, we need to retrieve the stock's with a service, right? Lets to this...

_Disclaimer: browser code..._

public class StockService : IStockService
{
    private readonly IHttpClient _httpClient;

    public StockService(IHttpClient httpClient)
    {
        _httpClient = httpClient; // Optional.
    }

    public async Task<IEnumerable<StockResults>> GetStocksAsync(IEnumerable<string> stockCodes)
    {
        var queryString = ConvertStockCodeListIntoQuerystring();
        string jsonResult;
        using (var httpClient = _httpClient ?? new HttpClient())
        {
            jsonResult = await httpClient.GetStringAsync("http:\\www.stocksOnline.com\stockResults?" + queryString);
        }

        return JsonConvert.DeserializeObject<StockResult>(jsonResult);
    }
}

Ok, so here we have a simple service that says: "_Go get me the stock prices for stocks AAA and BBB_";

So, I need to test this out:

• Given some legit stock names
• Given some invalid/non-existant stock names
• httpClient throws an exception (the internet blew up while waiting for a response)

and now I can easily test for those scenario's. I notice that when the httpClient blows up (throws an exception) .. this service blows up.. so maybe I need to add some logging and return null? donno .. but at least I can think about the problem and then handle it.

Of course - i 100% don't want to really hit the real web service during my normal unit tests. By injecting the mock, I can use that. Otherwise I can create a new instance and use that.

So i'm trying to establish that my suggestion is a hellava lot easier (to read/maintain/etc) that the current status quo.

In most common scenarios functionality provided by HttpClient will already be wrapped in some class such as HtmlDownloader.DownloadFile().

Why should we wrap HttpClient? It's _already_ an abstraction over all the http :sparkles: underneath. It's a great library! I personally don't see what that would achieve?

[tucaz]

@PureKrome your usage example is exactly what I mean by wrapping the functionality in a wrapping class.

I'm not sure I can understand what you are trying to test, but for me looks like you are testing the underlying webservice when what you really need to assert is that whoever is consuming it will be able to handle whatever comes back from the GetStockAsync method.

Please consider the following:

private IStockService stockService;

[Setup]
public void Setup()
{
    stockService = A.Fake<IStockService>();

    A.CallTo(() => stockService.GetStocksAsync(new [] { "Ticker1" }))
        .Returns(new StockResults[] { new StockResults { Price = 100m, Ticker = "Ticker1", Status = "OK" }});
    A.CallTo(() => stockService.GetStocksAsync(new [] { "Ticker2" }))
        .Returns(new StockResults[] { new StockResults { Price = 0m, Ticker = "Ticker2", Status = "NotFound" }});
    A.CallTo(() => stockService.GetStocksAsync(new [] { "Ticker3" }))
        .Throws(() => new InvalidOperationException("Some weird message"));
}

[Test]
public async void Get_Total_Stock_Quotes()
{
    var stockService = A.Fake<IStockService>();

    var total = await stockService.GetStockAsync(new [] { "Ticker1" });

    Assert.IsNotNull(total);
    Assert.IsGreaterThan(0, total.Sum(x => x.Price);
}

[Test]
public async void Hints_When_Ticker_Not_Found()
{
    var stockService = A.Fake<IStockService>();

    var total = await stockService.GetStockAsync(new [] { "Ticker2" });

    Assert.IsNotNull(total);
    Assert.AnyIs(x => x.Status == "NotFound");
}

[Test]
public async void Throws_InvalidOperationException_When_Error()
{
    var stockService = A.Fake<IStockService>();

    Assert.Throws(() => await stockService.GetStockAsync(new [] { "Ticker3" }), typeof(InvalidOperationException));
}

I'm proposing that you increase your abstraction one level up and deal with the actual business logic since there isn't anything that needs to be tested regarding the behavior of the GetStockAsync method.

The only scenario that I can think of needing to mock the HttpClient is when you need to test some retry logic depending on the response received from the server or something like that. In this case, you will be building a more complicated piece of code that will probably be extracted into a HttpClientWithRetryLogic class and not spread in every method you code that uses the HttpClient class.

If this is case, you can use the message handler as suggested or built a class to wrap all this retry logic because HttpClient will be a really small part of the important thing that your class does.

[MrJul]

@tucaz Didn't you just test that your fake returns what you configured, effectively testing the mocking library? @PureKrome wants to test the concrete StockService implementation.

The newly introduced level of abstraction should be wrapping HttpClient calls to be able to properly test StockService. Chances are that wrapper will simply be delegating every call to HttpClient, and add unnecessary complexity to the application.

You can always add a new layer of abstraction to hide non-mockable types. What's criticized in this thread is that mocking HttpClient is possible, but should probably be easier / in line of what's usually done with other libraries.

[tucaz]

@MrJul you are correct. That's why I said that what's been tested by his example is the webservice implementation itself and why I proposed (without code examples. My bad) that he mocks the IStockService so he can assert that the code consuming it is able to handle whatever the GetStockAsync returns.

That's what I think should be tested in the first place. Not the mocking framework (like I did) or the webservice implementation (like @PureKrome wants to do).

At the end of the day I can understand that it would be nice to have an IHttpClient interface, but fail to see any utility in these kinds of scenarios since IMO what's need to be tested is how consumers of the service abstraction (and therefore the webservice) can handle it's return appropriately.

[luisrudge]

@tucaz nope. Your test is wrong. You're testing the mocking library. Using @PureKrome's example, one would want to test three things:

• if the http request is using the right METHOD
• if the http request has the right URL
• if the response body can be successfully deserialized with a json deserializer

If you just mock IStockService in your Controller (for example), you aren't testing none of the three things mentioned above.

[tucaz]

@luisrudge agree with all your points. But... is the value of testing all these things you enumerate worth it? I mean, if I'm that committed in testing all those basic things that are covered by multiple frameworks (json.net, httpclient, etc) I think I can handle the way it is possible to do it today.

Still, if I want to make sure I can deserialize some weird response, can't I just make a test isolating those aspects of the code without the need to add HttpClient inside the test? Also, there is no way to prevent that the server will send the correct response you are expecting in your tests all the time. What if there is a response that is not covered? Your tests won't get it and you will have problems anyway.

My opinion is that such tests you are proposing have minimal net value and you should work on it only if you have the other 99% of your application covered. But at the end it is all a matter of personal preference and this is a subjective subject where different people see different value. And this, of course, don't mean that HttpClient shouldn't be more testable.

I think that the design of this specific piece of code took into account all these points and even if it is not the optimal outcome I don't think that the cost of changing it now would be lower than the possible benefits.

The objective I had in mind when I first spoke was to help @PureKrome try not to focus his efforts on what I perceive is a low value task. But again, it all comes down to personal opinion at some point.

[luisrudge]

Being sure you hit the correct url has low net value for you? What about being sure you do a PUT request to edit a resource instead of a post? The other 99% of my app can be covered, if this call's fails, then nothing will work.

[drub0y]

Soooo, I really think talking about HttpClient::GetStringAsync is a poor way to make a case for being able to mock HttpClient in the first place. If you're using the any of the HttpClient::Get[String|Stream|ByteArray]Async methods you've already lost because you can't do any proper error handling as everything is hidden behind an HttpRequestException that would be thrown and that doesn't even expose important details like the HTTP status code. So that means whatever abstraction you've built on top of the HttpClient will have no ability to react to and translate specific HTTP errors to domain specific exceptions and now you've got yourself a leaky abstraction... and one that gives your domain service's callers terrible exception information no less.

I think it's important to point this out because, in most cases, this means if you were using a mocking approach you'd be mocking the HttpClient methods that return HttpResponseMessage which already gets you to the same amount of work that is required to mock/fake HttpMessageHandler::SendAsync. In fact, it's more! You would potentially have to mock multiple methods (e.g. [Get|Put|Delete|Send]Async) at the HttpClient level instead of simply mocking SendAsync at the HttpMessageHandler level.

@luisrudge also mentioned deserializing JSON, so now we're talking about working with HttpContent which means you're working with one of the methods that return HttpResponseMessage for sure. Unless you're saying you're bypassing the entire media formatter architecture and deserializing strings from GetStringAsync into object instances manually with JsonSerializer::Deserialize or whatever. If that's the case then I don't think we can really have a conversation about _testing_ the API any more because that would just be using the API _itself_ completely wrong. :disappointed:

[tucaz]

@luisrudge it is important indeed, but you don't need to touch HttpClient to make sure it is right. Just make sure that the source where you read the URL from is returning things correctly and you are good to go. Again, not need to make a integration test with HttpClient in it.

These are important things, but once you get them right the chances of messing it up are really, really low unless you have a leaking pipe somewhere else.

[luisrudge]

@tucaz agree. Still: It's the most important part and I'd like to have it tested :)

[luisrudge]

@drub0y I'm amazed that using a public method is wrong :)

[drub0y]

@luisrudge Well, a different discussion I suppose, but I don't feel it's making the case for an interface that can be mocked by ignoring the inherent flaws in the use of these "convenience" methods in all but the most remedial of coding scenarios (e.g. utility apps and stage demos). If you choose to use them in production code, that's your choice, but you do so while hopefully acknowledging their shortcomings.

Anyway, let's get back to what @PureKrome wanted to see which is how one should design their classes to fit with the HttpClient API design such that they can easily test their code.

First, here's an example domain service that is going to be making HTTP calls to fulfill the needs of its domain:

``` c#
public interface ISomeDomainService
{
Task GetSomeThingsValueAsync(string id);
}

public class SomeDomainService : ISomeDomainService
{
private readonly HttpClient _httpClient;

public SomeDomainService() : this(new HttpClient())
{

}

public SomeDomainService(HttpClient httpClient)
{
    _httpClient = httpClient;
}

public async Task<string> GetSomeThingsValueAsync(string id)
{
    return await _httpClient.GetStringAsync("/things/" + Uri.EscapeUriString(id));
}

}

As you can see, it allows an `HttpClient` instance to be injected and also has a default constructor that instantiates the default `HttpClient` with no other special settings (obviously this default instance can be customized, but I'll leave that out for brevity).

Ok, so what's a test method look like for this thing using the MockHttp library then? Let's see:

``` c#
[Fact]
public async Task GettingSomeThingsValueReturnsExpectedValue()
{
    // Arrange
    var mockHttpMessageHandler = new MockHttpMessageHandler();
    mockHttpMessageHandler.Expect("http://unittest/things/123")
        .Respond(new StringContent("expected value"));

    SomeDomainService sut = new SomeDomainService(new HttpClient(mockHttpMessageHandler)
    {
        BaseAddress = new Uri("http://unittest")
    });

    // Act
    var value = await sut.GetSomeThingsValueAsync("123");

    // Assert
    value.Should().Be("expected value");
}

Err, that's pretty simple and straight forward; reads clear as day IMNSHO. Also, note that I'm leveraging FluentAssertions which, again, is another library I think a lot of us use and another strike against the "I don't want to have to bring in another library" argument.

Now, let me also illustrate why you'd probably never _really_ want to use GetStringAsync for any production code. Look back at the SomeDomainService::GetSomeThingsValueAsync method and assume it's calling a REST API that actually returns meaningful HTTP status codes. For example, maybe a "thing" with the id of "123" doesn't exist and so the API would return a 404 status. So I want to detect that and model it as the following domain specific exception:

``` c#
// serialization support excluded for brevity
public class SomeThingDoesntExistException : Exception
{
public SomeThingDoesntExistException(string id)
{
Id = id;
}

public string Id
{
    get;
    private set;
}

}

Ok, so let's rewrite the `SomeDomainService::GetSomeThingsValueAsync` method to do that now:

``` c#
public async Task<string> GetSomeThingsValueAsync(string id)
{
    try
    {
        return await _httpClient.GetStringAsync("/things/" + Uri.EscapeUriString(id));
    }
    catch(HttpRequestException requestException)
    {
        // uh, oh... no way to tell if it doesn't exist (404) or server maybe just errored (500)
    }
}

So that's what I was talking about when I said those basic "convenience" methods like GetStringAsync are not really "good" for production level code. All you can get is HttpRequestException and from there you cannot tell what status code you got and therefore cannot possibly translate it into the correct exception within a domain. Instead, you'd need to using GetAsync and interpret the HttpResponseMessage like so:

``` c#
public async Task GetSomeThingsValueAsync(string id)
{
HttpResponseMessage responseMessage = await _httpClient.GetAsync("/things/" + Uri.EscapeUriString(id));

if(responseMessage.IsSuccessStatusCode)
{
    return await responseMessage.Content.ReadAsStringAsync();
}
else
{
    switch(responseMessage.StatusCode)
    {
        case HttpStatusCode.NotFound:
            throw new SomeThingDoesntExistException(id);

        // any other cases you want to might want to handle specifically for your domain

        default:
            // Unhandled cases can throw domain specific lower level communication exceptions
            throw new HttpCommunicationException(responseMessage.StatusCode, responseMessage.ReasonPhrase);
    }
}

}

Ok, so now let's write a test to validate that I get my domain specific exception when I request a URL that doesn't exist:

``` C#
[Fact]
public void GettingSomeThingsValueForIdThatDoesntExistThrowsExpectedException()
{
    // Arrange
    var mockHttpMessageHandler = new MockHttpMessageHandler();

    SomeDomainService sut = new SomeDomainService(new HttpClient(mockHttpMessageHandler)
    {
        BaseAddress = new Uri("http://unittest")
    });

    // Act
    Func<Task> action = async () => await sut.GetSomeThingsValueAsync("SomeIdThatDoesntExist");

    // Assert
    action.ShouldThrow<SomeThingDoesntExistException>();
}

Ermahgerd it's even less code!!$!% Why? 'Cause I didn't even have to set up an expectation with MockHttp and it automagically returned me a 404 for the requested URL as a default behavior.

@PureKrome also mentioned a case where he wants to instantiate new HttpClient instances from within a service class. Like I said before, in that case you'd take a Func<HttpClient> as a dependency instead to abstract yourself from the actual creation or you could introduce your own factory interface if you're not a fan of Func for whatever reason.

[PureKrome]

I deleted my reply - i want to think about @drub0y 's reply and code a bit.

[luisrudge]

@drub0y you forgot to mention that someone will have to figure it out that one needs to implement a MockHttpMessageHandler in the first place. That's the whole point of this discussion. It isn't straight forward enough. With aspnet5, you can test everything just mocking public class from interfaces or base classes with virtual methods, but this specific API entry is different, less discoverable and less intuitive.

[drub0y]

@luisrudge Sure, I can agree with that, but how did we ever figure out that we needed a Moq or FakeItEasy? How did we figure out that we needed an N/XUnit, FluentAssertions, etc? I don't really see this as any different.

There are great (smart) people out there in the community who recognize the need for this stuff and actually take the time to start up libraries like these to solve these problems. (Kudos to all of them!) Then the solutions that are actually "good" get noticed by others who recognize the same needs and start grow organically through OSS and evangelization by the other leaders in the community. I think things are better than ever in this regard with the power of OSS finally being realized in the .NET community.

Now, that said, I do personally believe that Microsoft should have provided something _like_ MockHttp along with the delivery of the System.Net.Http APIs right "out of the box". The AngularJS team provided $httpBackEnd along with $http because they recognized people would absolutely _need_ this capability to be able to effectively test their apps. I think the fact that Microsoft _didn't_ identity this same problem is why there's so much "confusion" around best practices with this API and I think why we're all here having this discussion about it now. :disappointed: That said, I'm glad @richardszalay and @PureKrome have stepped up to the plate to try and fill that gap with their libraries and now I think we should get behind them to help improve their libraries however we can to make all of our lives easier. :smile:

[richardszalay]

Just got notified of this thread, so thought I'd throw my 2c in.

I use dynamic mocking frameworks as much as the next guy, but in recent years I've come to appreciate that some domains (like HTTP) are better served being faked behind something with more domain knowledge. Anything beyond basic URL matching would be pretty painful using dynamic mocking, and as you iterate on it you'd end up with your own mini version of what MockHttp does anyway.

Take Rx as another example: IObservable is an interface and therefore mockable, but it would be insanity to test complex compositions (not to mention schedulers) with dynamic mocking alone. The Rx testing library gives much more semantic meaning to the domain in which it works.

Back on HTTP specifically, albeit in the JS world, you can absolutely also use sinon to mock XMLHttpRequest, rather than use Angular's testing APIs, but I'd be pretty surprised if anyone did.

Having said all that, I totally agree about discoverability. Angular's HTTP testing documentation is right there with its HTTP documentation, but you'd need to know about MockHttp to look for it. As such, I'd be totally ok with contributing MockHttp back into the main library, if any of the CoreFx team wants to get in contact (the licenses are already the same).

[malixsys]

I like mocking it but we also use a Func signature:

public static async Task<T> GetWebObjectAsync<T>(Func<string, Task<string>> getHtmlAsync, string url)
{
    var html = await getHtmlAsync(url);
    var info = JsonConvert.DeserializeObject<T>(html);
    return info;
}

This allows for Test class code to be this simple:

Func<string, Task<string>> getHtmlAsync = u => Task.FromResult(EXPECTED_HTML);
var result = controller.InternalCallTheWeb(getHtmlAsync);

And the controller can simply do:

public static HttpClient InitHttpClient()
{
    return _httpClient ?? (_httpClient = new HttpClient(new WebRequestHandler
    {
        CachePolicy = new HttpRequestCachePolicy(HttpCacheAgeControl.MaxAge, new TimeSpan(0, 1, 0)),
        Credentials = new NetworkCredential(PublishingCredentialsUserName, PublishingCredentialsPassword)
    }, true));
}

[Route("Information")]
public async Task<IHttpActionResult> GetInformation()
{
    var httpClient = InitHttpClient();
    var result = await InternalCallTheWeb(async u => await httpClient.GetStringAsync(u));
    ...
}

But they COULD make it a bit easier... :)

[PureKrome]

Hi .NET team! Just touching base on this issue. It's been around for a month+ now and just curious to any thoughts on what the team thinks about this discussion.

There's been various points from both sides of the camp - all interesting and relevant IMO.

Are there any updates from you gals/guys?

ie.

1. We're read the convo and will not change a thing. Thanks for your input, have a lovely day. Here, have a piece of :cake:
2. We honestly haven't thought about it (things are hella-busy around here, as you could imagine) _but_ will think about it at a later date _before_ we go Release-To-Web (RTW).
3. Actually, we have had some internal discussions about it and we acknowledge that the public is finding it a wee bit painful to grok. So we are thinking we might do XXXXXXXXX.

ta!

[davidsh]

It's part 2 and 3.

One way to simplify the ability to insert a "mock" handler to help with testing is for a static method to be added to HttpClient, i.e. HttpClient.DefaultHandler. This static method will allow developers to set a different default handler than the current HttpClientHandler when calling the default HttpClient() constructor. In addition to helping with testing against a "mock" network, it will help developers that write code to portable libraries above HttpClient, where they don't directly create HttpClient instances. So, it would allow them to "inject" this alternate default handler.

So, this is one example of something we can add to the current object model that would be additive and not a breaking change to the current architecture.

[PureKrome]

Thanks heaps @davidsh for the very prompt reply :+1:

I'll happily wait and watch this space as it's exciting to know that it's not part 1 :smile: .. and look forward to seeing whatever ever changes happen :+1:

Thanks again (team) for your patience and listening -- really really appreciate it!

/me happily waits.

[luisrudge]

@davidsh so, the interface os virtual methods are out of question?

[davidsh]

It is not ouf of the question. But short term adding interfaces as was suggested in this issue thread needs careful study and design. It is not easily mapped to the existing object model of HttpClient and potentially would be a breaking change to the API surface. So, it is not something that is quick or easy to design/implement.

[luisrudge]

What about virtual methods?

[davidsh]

Yes, adding virtual methods is not a breaking change but an additive one. We are still working out what design changes are feasible in this area.

[PureKrome]

/me casts Resurrection upon this thread...

~ thread twists, groans, twiches and comes alive!

---

Just was listening to Aug20th 2015 Community StandUp and they mentioned that HttpClient will be avail cross plat with Beta7. YAY!

So ... has there been any decision's on this? Not suggesting either/or .. just politely asking for any discussion updates, etc?

[davidsh]

@PureKrome There has been no decision on this but this API design/investigation work is folded into our future plans. Right now, the majority of us are very focused on getting the rest of the System.Net code onto GitHub and working cross-platform. This includes both existing source code and porting our tests to the Xunit framework. Once this effort converges, the team will have more time to begin focusing on future changes like what is suggested in this thread.

[PureKrome]

Thanks heaps @davidsh - I really appreciate your time to reply :) Keep up the awesome work! :balloon:

[metzgithub]

Isn't using mocks for testing http services a bit obsolete? There are quite a few self hosting options what allow easy mocking of the http service and requires no code changes, only a change in service url. I.e. HttpListener and OWIN self host, but for complex API one could even build a mock service or use an autoresponder.

[PureKrome]

[reyou]

Any update on unit-test-ability of this HttpClient class? I am trying to Mock DeleteAsync method but as mentioned earlier Moq is complaining about function is not virtual. Seems there is no easier way other than creating our own wrapper.

[YehudahA]

I vote for Richardszalay.Mockhttp!
It's excellent! Simply and brilliant!

https://github.com/richardszalay/mockhttp

[PureKrome]

but @YehudahA - we shouldn't need that.... (that's the point of this convo) :)

[YehudahA]

@PureKrome we shouldn't need IHttpClient.
It's the same idea as EF 7. You never use IDbContext or IDbSet, but you just change the behavior using DbContextOptions..

Richardszalay.MockHttp is an out-of-the-box solution.

[JonHanna]

It's an out-of-another-box solution.

[PureKrome]

we shouldn't need IHttpClient.
It's the same idea as EF 7. You never use IDbContext or IDbSet, but you just change the behavior using DbContextOptions.

That's ok - I totally disagree, so we agree to disagree then.

Do both ways work? yes. Some people (like myself) find mocking an interface or virtual method EASIER for reasons like discoverability and readability and simplisity.

Others (like yourself) like to manipulate the behaviour. I find that more complex, harder to discover and more time consuming. (which (to me) translates to harder to support/maintain).

Each to their own, I guess :)

[YehudahA]

Richardszalay says: "This pattern is heavily inspired by AngularJS's $httpBackend".
That's right. Let see here: https://docs.angularjs.org/api/ngMock/service/$httpBackend

[metzgithub]

@PureKrome
Yes it is simpler.
You don't have to know what classes and methods are used, you don't need a mock library/interfaces/virtual methods, the setup will not change if you switch from HttpClient to something else.

        private static IDisposable FakeService(string uri, string response)
        {
            var httpListener = new HttpListener();
            httpListener.Prefixes.Add(uri);

            httpListener.Start();

            httpListener.GetContextAsync().ContinueWith(task =>
            {
                var context = task.Result;

                var buffer = Encoding.UTF8.GetBytes(response);

                context.Response.OutputStream.Write(buffer, 0, buffer.Length);

                context.Response.OutputStream.Close();
            });

            return httpListener;
        }

Usage:

            var uri = "http://localhost:8888/myservice/";
            var fakeResponse = "Hello World";

            using (FakeService(uri, fakeResponse))
            {
                // Run your test here ...

                // This is only to test that is really working:
                using (var client = new HttpClient())
                {
                    var result = client.GetStringAsync(uri).Result;
                }
            }

[DinisCruz]

@metzgithub is the best one here, since it allows for proper simulation of the behaviour of the target service.

It is exactly what I was looking for since it gives me the perfect place to put 'mal-formed/malicious' data (for security and non-happy-path tests)

[testpossessed]

I'm late to this thread, I arrived here after searching to see how people unit test their dependence on HttpClient, and found some very inventive ways of doing so. I will state up front that in this argument I would come down firmly on the side for having in interface. However the rub for me is that I don't have a choice.

I choose to design and develop components and services that declare their dependency on other components. I strongly prefer the choice to declare those dependencies via a constructor and at run time rely on an IoC container to inject a concrete implementation and control at this point whether it is a singleton or a transient instance.

I choose to use a definition of testability that I learned around a decade ago while embracing the then relatively new practice of Test Driven Development:

"A class is testable if it can be removed from it's normal environment and still work when all it's dependencies are injected"

Another choice I prefer to make is to test the interaction of my components with those they depend on. If I declare a dependency on a component I want to test that my component uses it correctly.

IMHO a well designed API allows me to work and code the way I choose, some technical constraints are acceptable. I see having only a concrete implementation of a component like HttpClient as depriving me of choice.

So I am left with the choice to resort to my usual solution in these cases and create a lightweight adapter/wrapper library that gives me an interface and allows me to work the way I choose.

[jdcrutchley]

I'm a little late to the party too, but even after reading all of the preceding comments, I'm still not sure how to proceed. Can you please tell me how I would verify that particular methods in my object under test call particular endpoints on a public API? I don't care what comes back. I just want to make sure that when someone calls "GoGetSomething" on an instance of my ApiClient class, that it sends a GET to "https://somenifty.com/api/something". What I'd LIKE to do is (not working code):

Mock<HttpClient> mockHttpClient = new Mock<HttpClient>();
mockHttpClient.Setup(x => x.SendAsync(It.IsAny<HttpRequestMessage>())).Verifiable();
ApiClient client = new ApiClient(mockHttpClient.Object);

Something response = await client.GoGetSomething();
mockHttpClient
    .Verify(x => x.SendAsync(
        It.Is<HttpRequestMessage>(m =>
            m.Method.Equals(HttpMethod.Get) &&
            m.RequestUri.Equals("https://somenifty.com/api/something"))));

But, it doesn't let me mock SendAsync.

IF I could get that basic thing working, then I might want to actually test how my class handles particular RETURNS from that call as well.

Any ideas? This doesn't seem (to me) like too much of an out-of-whack thing to want to test.

[davidsh]

But, it doesn't let me mock SendAsync.

You don't mock HttpClient.SendAsync method.

Instead you create a mock handler derived from HttpMessageHandler. Then you override the SendAsync method of that. Then you pass that handler object into the constructor of HttpClient.

[YehudahA]

@jdcrutchley, You can do it with @richardszalay's MockHttp.

public class MyApiClient
{
    private readonly HttpClient httpClient;

    public MyApiClient(HttpMessageHandler handler)
    {
        httpClient = new HttpClient(handler, disposeHandler: false);
    }

    public async Task<HttpStatusCode> GoGetSomething()
    {
        var response = await httpClient.GetAsync("https://somenifty.com/api/something");
        return response.StatusCode;
    }
}

[TestMethod]
public async Task MyApiClient_GetSomething_Test()
{
    // Arrange
    var mockHandler = new MockHttpMessageHandler();
    mockHandler.Expect("https://somenifty.com/api/something")
        .Respond(HttpStatusCode.OK);

    var client = new MyApiClient(mockHandler);

    // Act
    var response = await client.GoGetSomething();

    // Assert
    Assert.AreEqual(response, HttpStatusCode.OK);
    mockHandler.VerifyNoOutstandingExpectation();
}

[PureKrome]

@jdcrutchley or you create your OWN wrapper class and wrapper interface with one method SendAsync .. which just calls the _real_ httpClient.SendAsync (in your wrapper class).

I'm guessing that's what @richardszalay 's MockHttp is doing.

That's the general pattern people do when they can't mock something (because there's no interface OR it's not virtual).

[richardszalay]

mockhttp is a mock DSL on HttpMessageHandler. It's no different to dynamically mocking it using Moq, but the intent is clearer due to the DSL.

The experience is the same either way: mock HttpMessageHandler and create a concrete HttpClient from it. Your domain component depends on either HttpClient or HttpMessageHandler.

Edit: TBH, I'm not entirely sure what the problem with the existing design is. The difference between "mockable HttpClient" and "mockable HttpMessageHandler" is literally one line of code in your test: new HttpClient(mockHandler), and it has the advantage that the implementation maintains the same design for "Different implementation per platform" and "Different implementation for testing" (ie. is not tainted for the purposes of test code).

The existence of libraries like MockHttp only serves to expose a cleaner, domain-specific, test API. Making HttpClient mockable via an interface would not change this.

[KenRoytman]

As a newcomer to the HttpClient API, hopefully my experience will add value to the discussion.

I was also surprised to learn that HttpClient did not implement an interface for easy mocking. I searched around the web, found this thread, and read it from start to finish before proceeding. The answer from @drub0y convinced me to proceed down the route of simply mocking HttpMessageHandler.

My mocking framework of choice is Moq, probably pretty standard for .NET devs. After spending close to an hour fighting the Moq API with trying to setup and verify the protected SendAsync method, I finally hit a bug in Moq with verifying protected generic methods. See here.

I decided to share this experience to support the argument that a vanilla IHttpClient interface would've made life much easier and saved me a couple of hours. Now that I've hit a dead-end, you can add me to the list of devs that have written a simple wrapper/interface combo around HttpClient.

[richardszalay]

That's an excellent point Ken, I had forgotten that HttpMessageHandler.SendAsync was protected.

Still, I stand by my appreciation that the API design is focused on platform extensibility first, and it's not difficult to create a subclass of HttpMessageHandler (see mockhttp's as an example). It could even be a subclass that hands off the call to an abstract public method that is more Mock-friendly. I realise that seems "impure", but you could argue testibility vs tainting-API-surface-for-testing-purposes forever.

[scott-martin]

+1 for the interface.

Like others, I am a newcomer to HttpClient and need it in a class, started searching on how to unit test it, discovered this thread - and now several hours later I effectively have had to learn about it's implementation details in order to test it. The fact the so many have had to travel this road is proof that this is a real problem for people. And of the few that have spoken up, how many more are just reading the thread in silence?

I am in the middle of leading a large project at my company and simply do not have the time to chase down these sort of rabbit trails. An interface would have saved me time, my company money - and that is probably the same for many others.

I will probably go the route of creating my own wrapper. Because looking from the outside, it is not going to make sense to anyone why I injected an HttpMessageHandler into my class.

If it is not possible to create an interface for HttpClient - then perhaps the .net team can create a whole new http solution that does have an interface?

[YehudahA]

@scott-martin, HttpClient does nothing, he just sends the requests to HttpMessageHandler.

Instead of test the HttpClient you can test and mock the HttpMessageHandler. It's very simple, and you can also use richardszalay mockhttp.

[richardszalay]

@scott-martin as was mentioned only few comments up, you don't inject HttpMessageHandler. You inject HttpClient, and inject _that_ with HttpMessageHandler when you are unit testing.

[scott-martin]

@richardszalay thanks for the tip, that does work better at keeping low-level abstractions out of my implementations.

@YehudahA, I was able to get it working by mocking the HttpMessageHandler. But "getting it working" is not why I chimed in. As @PureKrome said, this is a problem of discover-ability. If there was an interface, I would have been able to mock it and be on my way in a matter of minutes. Because there isn't one, I had to spend several hours tracking down a solution.

I understand the limitations and hesitations to providing an interface. I just want to give my feedback as a member of the community that we like interfaces and please give us more of them - they are much easier to interact with and test against.

[PureKrome]

this is a problem of discover-ability. If there was an interface, I would have been able to mock it and be on my way in a matter of minutes. Because there isn't one, I had to spend several hours tracking down a solution.

:arrow_up: this. It's all about, this.

EDIT: emphasis, mine.

[richardszalay]

While I completely agree that it could be more discoverable, I'm not sure how it could take "several hours". Googling "mock httpclient" brings up this Stack Overflow question as the first result, in which the two highest rated answers (though, granted, not the accepted answer) describe HttpMessageHandler.

[PureKrome]

NP - we still agree to disagree. Tis All good :)

[luisrudge]

you can close this issue, since mscorlib is coming back with all it's baggage :)

[dasjestyr]

Can we just have these httpclient classes implement an interface? Then can deal with the rest on our own.

[colceagus]

Which solution do you propose for mocking the HttpClient used in a DI Service ?
I'm trying to implement a FakeHttpClientHandler and I don't know what to do with the SendAsync method.

I have a simple GeoLocationService that calls a microservice for retrieving ip based location information from another one. I want to pass the fake HttpClientHandler to the service, which has two constructors, one receiving the locationServiceAddress and one with the locationServiceAddress and the additional HttpClientHandler.

public class GeoLocationService: IGeoLocationService {

        private readonly string _geoLocationServiceAddress;
        private HttpClient _httpClient;
        private readonly object _gate = new object();

        /// <summary>
        /// Creates a GeoLocation Service Instance for Dependency Injection
        /// </summary>
        /// <param name="locationServiceAddress">The GeoLocation Service Hostname (IP Address), including port number</param>
        public GeoLocationService(string locationServiceAddress) {
            // Add the ending slash to be able to GetAsync with the ipAddress directly
            if (!locationServiceAddress.EndsWith("/")) {
                locationServiceAddress += "/";
            }

            this._geoLocationServiceAddress = locationServiceAddress;
            this._httpClient = new HttpClient {
                BaseAddress = new Uri(this._geoLocationServiceAddress)
            };
        }

        /// <summary>
        /// Creates a GeoLocation Service Instance for Dependency Injection (additional constructor for Unit Testing the Service)
        /// </summary>
        /// <param name="locationServiceAddress">The GeoLocation Service Hostname (IP Address), including port number.</param>
        /// <param name="clientHandler">The HttpClientHandler for the HttpClient for mocking responses in Unit Tests.</param>
        public GeoLocationService(string locationServiceAddress, HttpClientHandler clientHandler): this(locationServiceAddress) {
            this._httpClient.Dispose();
            this._httpClient = new HttpClient(clientHandler) {
                BaseAddress = new Uri(this._geoLocationServiceAddress)
            };
        }

        /// <summary>
        /// Geo Location Microservice Http Call with recreation of HttpClient in case of failure
        /// </summary>
        /// <param name="ipAddress">The ip address to locate geographically</param>
        /// <returns>A <see cref="string">string</see> representation of the Json Location Object.</returns>
        private async Task<string> CallGeoLocationServiceAsync(string ipAddress) {
            HttpResponseMessage response;
            string result = null;

            try {
                response = await _httpClient.GetAsync(ipAddress);

                response.EnsureSuccessStatusCode();

                result = await response.Content.ReadAsStringAsync();
            }
            catch (Exception ex) {
                lock (_gate) {
                    _httpClient.Dispose(); 
                    _httpClient = new HttpClient {
                        BaseAddress = new Uri(this._geoLocationServiceAddress)
                    };
                }

                result = null;

                Logger.LogExceptionLine("GeoLocationService", "CallGeoLocationServiceAsync", ex.Message, stacktrace: ex.StackTrace);
            }

            return result;
        }

        /// <summary>
        /// Calls the Geo Location Microservice and returns the location description for the provided IP Address. 
        /// </summary>
        /// <param name="ipAddress">The <see cref="string">IP Address</see> to be geographically located by the GeoLocation Microservice.</param>
        /// <returns>A <see cref="string">string</see> representing the json object location description. Does two retries in the case of call failure.</returns>
        public async Task<string> LocateAsync(string ipAddress) {
            int noOfRetries = 2;
            string result = "";

            for (int i = 0; i < noOfRetries; i ++) {
                result = await CallGeoLocationServiceAsync(ipAddress);

                if (result != null) {
                    return result;
                }
            }

            return String.Format(Constants.Location.DefaultGeoLocationResponse, ipAddress);
        }

        public void Dispose() {
            _httpClient.Dispose();
        }

    }

In the test class, I want to write an internal class named FakeClientHandler which extends HttpClientHandler, that overrides the SendAsync Method. Another method, would be, I guess, to use a mock framework to mock the HttpClientHandler. I don't know yet how to do that. If you could help, I would be forever in your debt.

public class GeoLocationServiceTests {

        private readonly IConfiguration _configuration;
        private IGeoLocationService _geoLocationService;
        private HttpClientHandler _clientHandler;

        internal class FakeClientHandler : HttpClientHandler {

            protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) {

                // insert implementation here

                // insert fake responses here. 
                return (Task<HttpResponseMessage>) null;
            }

        }

        public GeoLocationServiceTests() {
            this._clientHandler = new FakeClientHandler();
            this._clientHandler.UseDefaultCredentials = true;
            this._geoLocationService = new GeoLocationService("http://fakegeolocation.com/json/", this._clientHandler);
        }


        [Fact]
        public async void RequestGeoLocationFromMicroservice() {
            string ipAddress = "192.36.1.252";

            string apiResponse = await this._geoLocationService.LocateAsync(ipAddress);
            Dictionary<string, string> result = JsonConvert.DeserializeObject<Dictionary<string,string>>(apiResponse);

            Assert.True(result.ContainsKey("city"));

            string timeZone;
            result.TryGetValue("time_zone", out timeZone);

            Assert.Equal(timeZone, @"Europe/Stockholm");
        }

        [Fact]
        public async void RequestBadGeoLocation() {
            string badIpAddress = "asjldf";

            string apiResponse = await this._geoLocationService.LocateAsync(badIpAddress);
            Dictionary<string, string> result = JsonConvert.DeserializeObject<Dictionary<string, string>>(apiResponse);

            Assert.True(result.ContainsKey("city"));

            string city;
            result.TryGetValue("city", out city);
            Assert.Equal(city, "NA");

            string ipAddress;
            result.TryGetValue("ip", out ipAddress);
            Assert.Equal(badIpAddress, ipAddress);
        }
    }

[richardszalay]

@danielmihai The HttpMessageHandler is effectively the "implementation". You typically create an HttpClient, passing the handler into the constuctor, and then have your service layer depend on HttpClient.

Using a generic mocking framework is difficult because the overridable SendAsync is protected. I wrote mockhttp specifically for that reason (but also because it's nice to have domain-specific mocking features.

[PureKrome]

@danielmihai What Richard said pretty much sums up the _pain_ we all have to endure right now with mocking HttpClient.

There's also HttpClient.Helpers as _another_ library which helps you mock out HttpClient .. or more specifically, provide a fake HttpMessageHandler for you to fake up the responses.

[colceagus]

I've figured it out, written something like this... [I think I've dealt with the 'pain']

internal class FakeClientHandler : HttpClientHandler {
            protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) {
                if (request.Method == HttpMethod.Get) {
                    if (request.RequestUri.PathAndQuery.Contains("/json/")) {
                        string requestPath = request.RequestUri.PathAndQuery;
                        string[] splitPath = requestPath.Split(new char[] { '/' });
                        string ipAddress = splitPath.Last();

                        // RequestGeoLocationFromMicroservice
                        if (ipAddress.Equals(ipAddress1)) {
                            var response = new HttpResponseMessage(HttpStatusCode.OK);
                            response.Content = new StringContent(response1);

                            return Task.FromResult(response);
                        }

                        // RequestBadGeoLocation
                        if (ipAddress.Equals(ipAddress2)) {
                            var response = new HttpResponseMessage(HttpStatusCode.OK);
                            response.Content = new StringContent(response2);
                            return Task.FromResult(response);
                        }
                    }
                }
                return (Task<HttpResponseMessage>) null;
            }
        }

All tests pass

Please do comment on the implementation, if there are better ways to do it.

Thanks!

[Thaoden]

Adding my thoughts to this more than a year old discussion.

@SidharthNabar

First, I notice that you are creating new instances of HttpClient for sending each request - that is not the intended design pattern for HttpClient. Creating one HttpClient instance and reusing it for all your requests helps optimize connection pooling and memory management. Please consider reusing a single instance of HttpClient. Once you do this, you can insert the fake handler into just one instance of HttpClient and you're done.

Where do I dispose of the not-injected instance when I'm not supposed to use a using statement, but new up my HttpClient instance eg in the consumer's constructor? What about the HttpMessageHandler that I'm constructing for injecting into the HttpClient instance?

The complete interface-or-not-discussion aside (I'm all for interfaces), at least the docs could state that you can inject a HttpMessageHandler for testing purposes. Yes, the constructor is mentioned, yes, it is mentioned that you can inject your own HttpMessageHandler, but nowhere is it stated why I would want to do this.

@richardszalay

While I completely agree that it could be more discoverable, I'm not sure how it could take "several hours". Googling "mock httpclient" brings up this Stack Overflow question as the first result, in which the two highest rated answers (though, granted, not the accepted answer) describe HttpMessageHandler.

An API should be intuitive to use with browsing through the docs, IMHO. Having to ask Google or SO question is certainly not intuitive, the linked question dating back to way before .NET Core makes the up-to-dateness not obvious (maybe something changed in between the question and the release of a _new_ framework?)

Thanks to everyone who's added their code examples and provided helper libs!

[dasjestyr]

I mean, if we have a class that needs to make an http call, and we want to unit test that class, then we have to be able to mock that http client. Since http client isn't interfaced, I can't inject a mock for that client. Instead, I have to build a wrapper (that implements an interface) for http client that accepts a message handler as an optional parameter, and then send it a fake handler and inject that. That's a whole lot of extra hoops to jump through for nothing. The alternative is to have my class take an instance of HttpMessageHandler instead of HttpClient, I guess...

I don't mean to split hairs, it just feels awkward and seems a bit unintuitive. I think the fact that this is such a popular topic sort of supports that.

[shaunluttin]

Since http client isn't interfaced, we cant inject it with a container.

@dasjestyr We can do dependency injection without interfaces. Interfaces enable multiple inheritance; they are not a requirement for dependency injection.

[PureKrome]

Yeah, DI is not the issue here. It's how easy/nice it is to mock it out, etc.

[dasjestyr]

@shaunluttin I'm aware that we can do DI without an interface, I didn't imply otherwise; I'm speaking of a class that has a dependency on an HttpClient. Without the interface for the client, I can't inject a mocked http client or any http client (so that it can be tested in isolation without making an actual http call) unless I wrap it in an adapter that implements an interface that I've define to mimic that of the HttpClient. I've changed some wording slightly for clarity in case you were fixating on a single sentence.

In the meanwhile, I've been creating a fake message handler in my unit tests and injecting that which means my classes are designed around injecting a handler which I guess isn't the worst thing in the world but it feels awkward. And the tests need fakes instead of mocks/stubs. Gross.

Also, speaking academically, interfaces do not enable multiple inheritance, they only allow you to mimic it. You don't inherit interfaces, you implement them -- you're not _inheriting_ any functionality by implementing interfaces, only declaring that you have implemented the functionality in some way. To otherwise mimic the multiple inheritance and some already existing functionality that you've written elsewhere, you might implement the interface and pipe the implementation to an internal member that holds the actual functionality (e.g. an adapter) which is actually composition, not inheritance.

[PureKrome]

In the meanwhile, I've been creating a fake message handler in my unit tests and injecting that which means my classes are designed around injecting a handler which I guess isn't the worst thing in the world but it feels awkward. And the tests need fakes instead of mocks/stubs. Gross.

This is heart/core of this issue -> and it's _purely_ opinionated too.

One side of the fence: fake message handler because <insert their rational here>
Other side of the fence: interfaces (or even virtual methods, but that still is a tiny bit of a PITA) because of <insert their rational here>.

[dasjestyr]

Well I mean fakes take a lot of extra work to set up and in many cases result in you writing a whole other class just for the test that doesn't really add a lot of value. Stubs are great because it'll allow the test to continue without a real implementation. Mocks are what I'm really interested because they can be used for assertions. For example, assert that the method on this interface's implementation was called x amount of times with this test's current setup.

I actually had to test that on one of my classes. To do it, I had to add an accumulator to the fake handler that incremented each time the SendAsync() method was called. That was a simple case, thankfully, but come on. Mocking is practically a standard at this point.

[PureKrome]

Yep - totally agree with you here 👍 🍰

[lasseschou]

Please, Microsoft, add the IHttpClient, period. OR be consistent and remove all your interfaces in the BCL - IList, IDictionary, ICollection, because they are "hard to version" anyway. Heck, also get rid of IEnumerable.

On a serious note, instead of arguing like "you can just write your own handler" (sure, but an interface is easier and cleaner) and "an interface introduces breaking change" (well, you do that in all .net versions anyway), simply ADD THE INTERFACE, and let developers decide how they want to test it.

[jnm2]

@lasseschou IList, IDictionary, ICollection and IEnumerable are absolutely critical because of the wide variety of classes that implement them. IHttpClient would only be implemented by HttpClient- to be consistent with your logic, every single class in the BCL would have to be given an interface type because we might need to mock it.
I would be sad to see this kind of clutter.

Unless there is a dramatic tradeoff, IHttpClient is the wrong abstraction level to be mocking anyhow. Your app will almost certainly not be exercising the full range of HttpClient's capabilities. I would suggest that you wrap HttpClient in your own service in terms of your app's specific requests and mock your own service.

[PureKrome]

@jnm2 - Out of interest, why do you think an IHttpClient is the wrong abstraction level to be mocking?

I'm not hating or trolling - honest Q.

(Like to learn, etc).

[dasjestyr]

I was gonna ask the same thing considering its pretty much a perfect level of abstraction seeing as it is a client.

[jnm2]

It's not an absolute. If you're building a web browser, then I could see how it's 1:1 between what your app needs and what HttpClient abstracts. But if you're using HttpClient to talk to an API of some sort- anything with more specific expectations on top of HTTP- you're only going to need a fraction of what HttpClient can do. If you build a service which encapsulates the HttpClient transport mechanism and mock that service, the rest of your app can code against your service's API-specific interface abstraction, rather than coding against HttpClient.

Also, I subscribe to the "Never mock a type you don't own" principle, so YMMV.
See also http://martinfowler.com/articles/mocksArentStubs.html comparing classical testing vs mocking.

[lasseschou]

The point here is that the HttpClient is such a great advancement over
WebClient, XMLHttpRequest, that it was become the preferred way to do HTTP
calls. It's clean and modern. I always use mockable adapters that talk
directly to the HttpClient, so testing my code is easy. But I'd like to
test those adapter classes as well. And that's when I discover: there's no
IHttpClient. Perhaps you're right, it's not needed. Perhaps you think it's
the wrong level of abstraction. But it would be really convenient not
having to wrap it in yet another class in all projects. I just don't see
any reason not to have a separate interface for such an important class.

Lasse Schou
Founder & CEO

http://mouseflow.com
[email protected]

https://www.facebook.com/mouseflowdotcom/
https://twitter.com/mouseflow
https://www.linkedin.com/company/mouseflow

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
for the sole use of the intended recipient(s) and may contain confidential,
proprietary, and/or privileged information protected by law. Any
unauthorized review, use, disclosure or distribution is prohibited. If you
are not the intended recipient, please contact the sender by reply e-mail
and destroy all copies of the original message.

On 12 November 2016 at 14:57, Joseph Musser [email protected]
wrote:

It's not an absolute. If you're building a web browser, then I could see
how it's 1:1 between what your app needs and what HttpClient abstracts. But
if you're using HttpClient to talk to an API of some sort- anything with
more specific expectations on top of HTTP, you're only going to need a
fraction of what HttpClient can do. If you build a service which
encapsulates the HttpClient transport mechanism and mock that service, the
rest of your app can code against your service's API-specific interface
abstraction, rather than coding against HttpClient.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/1624#issuecomment-260123552, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACnGPp3fSriEJCAjXeAqMZbBcTWcRm9Rks5q9cXlgaJpZM4EPBHy
.

[brphelps]

I'm hearing some weird input that the HttpClient is "too flexible" to be the right layer to mock -- That seems more like a side effect of the decision to have one class have so many responsibilities and exposed features. But it's really independent of how people _want_ to be testing the code.

I'm still really hopeful we fix this, if only because we're breaking encapsulation by expecting consumers to know these details of how HttpClient is implemented (In this case, to "know" that it's a wrapper that doesn't add meaningful functionality and wraps another class which they may or may not be using). Framework consumers shouldn't have to know anything about the implementation than exactly what interface is exposed to them, and a lot of the rationalization here is that we have a workaround and that deep familiarity with the class offers many not-well-encapsulated ways of mocking / stubbing functionality.

Fix please!

[damianh]

Just stumbled upon this so throwing my 2c in...

I think mocking HttpClient is folly.

Even as there is just a single SendAsync to mock, there are _so_ many nuances to HttpResponseMessage, HttpResponseHeaders, HttpContentHeaders, etc, your mocks are going to get messy fast and are probably behaving wrong too.

Me? I use OwinHttpMessageHandler to

1. Perform HTTP acceptance tests from the outside of a service.
2. Inject _Dummy_ HTTP services into components that want to make outbound HTTP requests to said services. That is, my component has an optional ctor parameter for HttpMessageHandler.

I'm sure there is an MVC6 handler that does similar....

_Life is fine._

[brphelps]

@damianh: Why should people using dotnet core need to do anything with Owin to test a class like HttpClient? This might make a lot of sense for a particular use case (AspCore hosted on OWIN) but this doesn't seem to be that general.

This really seems to be conflating the concern of "what's the most general way to Unit Test HttpClient that is also consistent with user expectations" with "how do I integration test" :).

[damianh]

Oh _testing_ HttpClient... thought the topic was testing things that have a
dependency on HttpClient. Don't mind me. :)

On 5 Dec 2016 7:56 p.m., "brphelps" [e-mail] wrote:

@damianh https://github.com/damianh: Why should people using dotnet core
need to do anything with Owin to test a class like HttpClient? This might
make a lot of sense for a particular use case (AspCore hosted on OWIN) but
this doesn't seem to be that general.

This really seems to be conflating the concern of "what's the most general
way to Unit Test HttpClient that is also consistent with user expectations"
with "how do I integration test" :).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/1624#issuecomment-264942511, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADgXJxunxBgFGLozOsisCoPqjMoch48ks5rFF5vgaJpZM4EPBHy
.

[brphelps]

@damianh -- It is things that have a dependency, but your solution seems to involve a lot of other dependencies in the picture, too. I'm imagining a class library that happens to use a passed in HttpClient instance (or a DI'ed one)-- Where does Owin come into the picture?

[damianh]

It's not owin that is the interesting thing I was trying to show (which
will also work on .net core / netstandard) so please don't focus on that.

The interesting thing is the HttpMessageHandler that make an in-process
in-memory HTTP request against dummy HTTP endpoints.

Aspnet Core's TestServer, as did the katana one before it, works the same
way.

So your class under test has an HttpClient DI'd where _it_ has an
HttpMessageHandler injected into it that directly. The testable and
assertable surface is there.

Mocking, a specific form of test double, is something I'd still advise
against for HttpClient. And thus, no need for an IHttpClient.

On 5 Dec 2016 8:15 p.m., "brphelps" [e-mail] wrote:

@damianh https://github.com/damianh -- It is things that have a
dependency, but your solution seems to involve a lot of other dependencies
in the picture, too. I'm imagining a class library that happens to use a
passed in HttpClient instance (or a DI'ed one)-- Where does Owin come into
the picture?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/1624#issuecomment-264947538, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADgXEESfQj9NnKLo8LucFLyajWtQXKBks5rFGK8gaJpZM4EPBHy
.

[brphelps]

@damianh -- Totally understand what you're saying the interesting thing is -- but people don't need dummy HTTP endpoints with a smart mock-- e.g. @richardszalay's MockHttp library.

At least not for unit testing :).

[damianh]

It depends. Options are good :)

On 5 Dec 2016 8:39 p.m., "brphelps" [e-mail] wrote:

@damianh https://github.com/damianh -- Totally understand what you're
saying the interesting thing is -- but people don't need dummy HTTP
endpoints with a smart mock-- e.g. @richardszalay
https://github.com/richardszalay's MockHttp library.

At least not for unit testing :).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/1624#issuecomment-264954210, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADgXOl4UEDGYCVLpbvwhueaK52VtjH6ks5rFGhlgaJpZM4EPBHy
.

[PureKrome]

@damianh said:
Options are good :)

Yep - can totally agree on that 👍 Which, IMO .. we don't have right now 😢 **

I totally understand the rational for creating and injecting your own HMH. That's what I'm doing right now. _Unfortunately_, i have to create a 2nd ctor (or provide an optional ctor param) _for unit testing concerns_. Just think about that sentence again -> i'm creating an entirely possible USER OPTION ... which is really only to enable a unit test to _do stuff_. An end-user/consumer of my class will never _really_ use that ctor option ... which to me smells.

Back to the point about options -> right now I don't feel like we have one. We _have_ to _learn_ about the internals of HttpClient. Literally open up the bonnet and look under the hood. (I was lucky enough to get a TL;DR; from Sir Fowler The Awesome ™️ which cut my research time right down).

So - with your point about being no-interface, can you please provide some examples of when an interface would be a bad thing, please? etc. Remember -> I've always been promoting an interface as an _option_ for small/simple to medium examples ... not 100% of all examples.

I'm not trying to troll or attack you Damian - just trying to understand.

* Ninja Edit: technically, we actually do have options -> we can do nothing / create our own wrappers / etc. I was meaning: not many options that simplify things, *out-of-the-box.

[damianh]

Hey @PureKrome , no worries just sharing experience.

The term 'Unit Test' has been used multiple times and let's think about this for a minute. So we're on the same page, I'm assuming we have a FooClass that has a ctor dependency on HttpClient (or HMH) and we want to _mock_ HttpClient.

What we are really saying here is "FooClass can make an HTTP request using to any URL (protocol / host / resource), using any method, any content type, any transfer-encoding, any content-encoding, _any request headers_, and can handle any response content-type, any status code, cookies, redirects, transfer encoding, cache control headers, etc as well as network timeouts, task cancelled exceptions etc.".

HttpClient is like a God Object; you can do a lot with it. HTTP is also a remote service call reaching out beyond your process.

Is this really unit testing? Be honest now :)

** Ninja Edit too: If you want to make FooClass unit testable where it wanted to get some json from a server, it would have a ctor dependency on Func<CancellationToken, Task<JObject>> or similar.

[damianh]

can you please provide some examples of when an interface would be a bad thing, please

Didn't say that! I said IHttpClient would not be useful because A) it's won't be substituted with an alternative concrete implementation and B) the God Object concern as mentioned.

Another example? IAssembly.

[brphelps]

@damianh -- The "God Class" status isn't a concern to callers. It's not the callers' responsibility to make sure HttpClient is / isn't a god class. They know that it's the object they use to make HTTP calls :).

So, assuming that we accept that the .NET framework exposed a God class to us... how can we reduce our concern? How can we best encapsulate it? That's easy -- Use a traditional mocking framework (Moq is a great example) and completely isolate the HttpClient implementation, because as people consuming it, we just don't care how it's internally designed. We only care about how we use it. The problem here is that the conventional approach doesn't work, and as a result, you see people coming up with more and more interesting solutions to solve the problem :).

Some edits for grammar and clarity :D

[PureKrome]

Totally agree @damianh that HttpClient is like some uber God Class.

I also don't see why I need to know about all the internal's again because there can be _so many purmutations_ to what goes in, what comes out.

For example: I want to get the stocks from some API. https://api.stock-r-us.com/aapl. Result is some JSON paylod.

so, things i might screw up when trying to call that endpoint:

• wrong endpoint url?
• wrong scheme?
• missing particular header(s)?
• incorrect headers
• incorrect request payload (maybe this is a POST with some body?)
• network error/timeout/etc
  ...

So I understand that if I want to test for these things, the HMH is a great way to do this because it just hijacks the _real_ HMH and substitutes what I've said, to return.

But -- what about if I'm wanting (at my own risk) to ignore all of these things and just say:
_Imagine I want to call and endpoint, everything is OK and I get my nice json payload result._
Should I still learn and worry about all of this stuff, to do something as simple as this?

Or ... is this a poor way of thinking. We should never be thinking like this because it too hacky? too cheap? too prone to errors?

Most of the time I just call an endpoint. I don't think about things like headers or cache-control or network errors. I usually just think about the VERB + URL (and PAYLOAD, if required) and then the RESULT. I have error handling around this to Catch-All-The-Things :tm: and then admit defeat because something Bad happened :(

So i'm either looking at the problem wrong (ie. i'm still can't shake my n00b status) -or- this God Class is causing us a heap of unit testing grief.

HTTP is also a remote service call reaching out beyond your process.
Is this really unit testing? Be honest now :)

To me, HttpClient is a _dependant service_. I don't care _what_ the service really does, I classify it as _something that does some stuff which is external to my codebase_. Therefore, I don't want it doing stuff with an external-power so I want to control that, hijack it and determine the results of those service-calls. I'm still trying to test _my_ unit of work. My little piece of logic. Sure it might depend on other stuff. It's still a unit test if I can control the entire universe this piece of logic exists within _and_ I don't have rely/integrate on other services.

** Another edit: Or maybe - if HttpClient is actually a God Object, maybe the conversation should be around reducing it to something ... better? Or maybe, it's not a God Object at all? (just trying to open the debate up)..

[richardszalay]

Unfortunately, i have to create a 2nd ctor (or provide an optional ctor param) for unit testing concerns. Just think about that sentence again -> i'm creating an entirely possible USER OPTION ... which is really only to enable a unit test to do stuff. An end-user/consumer of my class will never really use that ctor option ... which to me smells

I'm not sure what you're trying to say here:

"I don't like having a second ctor that accepts HttpMessageHandler" ... so have your tests wrap it in an HttpClient

"I want to unit test without inverting my dependency on HttpClient" ... tough?

[brphelps]

@richardszalay -- I think I get the sentiment that @PureKrome doesn't want to change his code in a way that doesn't improve design just to support an overly opinionated test strategy. Don't get me wrong, I'm not complaining about your library, the only reason I want a better solution is because I have a higher expectation of dotnet core to not require it =).

[richardszalay]

I'm not thinking of this in terms of MockHttp, but testing/mocking in general.
Inverted dependencies is a fundamental necessity for unit testing in .NET, and that usually involves ctor-injection. I'm not clear on what the proposed alternative would be.

Surely if HttpClient implemented IHttpClient, there would still be a constructor that accepted it...

[PureKrome]

Surely if HttpClient implemented IHttpClient, there would still be a constructor that accepted it...

Agreed. I was "crossing streams" in my thinking as I literally just woke up and was struggling to get my single cell in my brain to come alive.

Ignore my 'ctor' comment plz and continue with the discussion, if anyone hasn't already left in boredom 😄

[Thaoden]

Can we take a step back to the question if using HttpClient as a dependency really _can_ lead to Unit Testing.

Let's say I have a class FooClass that gets a dependency constructor injected. I now want to Unit Test the methods on FooClass, meaning I specify the input to my method, I isolate FooClass as best as I can from its environment and I check the outcome against an expected value. In all this pipeline, I wouldn't want to care about the entrails of my dependent object; it would be best for me if I could just specify the kind of environment that my FooClass should live in: I would want to mock the dependency on my system under test in that way that it behaves exactly as I need it to for my tests to work.

Now currently I have to care about the way HttpClient is implemented because I have to specify a dependency (HttpMessageHandler) for my injected dependency (HttpClient) for my system under test. Instead of just mocking out my direct dependency, I have to create a concrete implementation of it with a mocked chained dependency.

[brphelps]

Theme: I can't mock HttpClient without knowing how its internals work, and I also have to rely on its implementation to not do anything surprising since I can't completely bypass the internals of the class via mocking. This is basically not following Unit Testing 101 -- getting unrelated dependency code out of your tests =).

You can see the theme present in many different comments:

"To me, HttpClient is a dependant service. I don't care what the service really does, I classify it as something that does some stuff which is external to my codebase. " -- PureKrome

"Now currently I have to care about the way HttpClient is implemented because I have to specify a dependency (HttpMessageHandler) for my injected dependency (HttpClient) for my system under test. " -- Thaoden

"I was gonna ask the same thing considering its pretty much a perfect level of abstraction seeing as it is a client." -- dasjestyr

"The HttpMessageHandler is effectively the "implementation". You typically create an HttpClient, passing the handler into the constuctor, and then have your service layer depend on HttpClient." -- richardszalay

" But the key issue will remain - you will need to define a fake Handler and insert it below the HttpClient object." -- SidharthNabar

"HttpClient has lots of options for unit testability. It's not sealed and most of its members are virtual. It has a base class that can be used to simplify the abstraction as well as a carefully designed extension point in HttpMessageHandler" -- ericstj

"If the API is not mockable, we should fix it. But it has nothing to do with interfaces vs classes. Interface is no different than pure abstract class from mockability perspective, for all practical purposes." -- KrzysztofCwalina

"With respect, if the option is having to write your own Faked Message Handler, which isn't obvious without digging through the code, then that is a very high barrier that is going to be hugely frustrating for a lot of developers." -- ctolkien

[dasjestyr]

Is this really unit testing? Be honest now :)

I dunno. What does the test look like?

I think the statement is a bit of a strawman. The point is to mock the dependency which is the http client, so that the code can run in isolation from those dependencies... in other words, not actually make an http call. All we'd be doing is make sure that the expected methods on the interface were being called under the right conditions. Whether or not we should have chosen a lighter dependency seems to be a completely different discussion.

[damianh]

HttpClient is the worst dependency ever. It literally states that the
dependee can "call the Internet". Sure y'all be mocking TCP next. Heh

On 7 Dec 2016 5:34 a.m., "Jeremy Stafford" [e-mail] wrote:

Is this really unit testing? Be honest now :)

I dunno. What does the test look like?

I think the statement is a bit of a strawman. The point is to mock the
dependency which is the http client. All we'd be doing is make sure that
the expected methods on the interface were being called under the right
conditions. Whether or not we should have chosen a lighter dependency seems
to be a completely different discussion.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dotnet/corefx/issues/1624#issuecomment-265353526, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADgXPL39vnUbWrUuUBtfmbjhk5IBkxHks5rFjdqgaJpZM4EPBHy
.

[dasjestyr]

This is basically not following Unit Testing 101 -- getting unrelated dependency code out of your tests =).

Personally, I don't think that's the right interpretation of that particular practice. We're not trying to test the dependency, we're trying to test how our code interacts with that dependency, thus it is not unrelated. And, quite simply, If I can't run a unit test against my code without invoking the actual dependency, then that's not testing in isolation. For example, if that unit test was run on a build server without internet access, then the test would likely break -- hence the desire to mock it. You could remove the dependency on HttpClient, but that just means it's going to end up in some other class that that you write which makes the call -- maybe I'm talking about that class? You don't know because you never asked. So should my domain service have a dependency on HttpClient? No, of course not. It'd be abstracted to some other sort of abstract service and I'd mock that instead. Ok, so in the layer that injects whatever abstraction I come up with -- somewhere up the chain, _something_ is going to know about HttpClient and that something is going to be wrapped by something else that I write, and that something else is something I'm still going to write a unit test for.

So, if my unit test for some kind of mediator is testing that a non-successful call to a service out on the internet results in my code rolling back some form of transaction vs. a successful call to a service resulting in the committing of that transaction, then I need to have control over the result of that call in order to set the conditions of that test so that I can assert that both scenarios are behaving correctly.

I could understand the opinion of some that HttpClient is not the correct level of abstraction, however I don't think anyone asked about the "level" of the consuming class, which is kind of presumptive. One could abstract the HTTP call to a service and interface that, but some of us still want to test that our "wrappers" for that HTTP call are behaving as expected, which means that we need control over the response. Currently, this can be achieved by giving HttpClient a fake handler which is what I'm doing now, and that's fine, but the point is that if you want that coverage, you're going to have to futz with HttpClient at some point to at least fake the response.

But, now let me backpedal. The more I look at it, it's far too fat for a mockable interface (which has been said). I think it may be a mental thing stemming from the name "Client". I'm not saying that I think it's wrong, but rather, it's a bit out of alignment with other "Client" implementations that one might see out there in the wild these days. We see a lot of "clients" out there today that are really service facades, so when one sees the name Http CLIENT, they may think the same thing and why would you not mock a service, right?

I see now that the handler is the part that matters to the code, so I'll be designing around that going forward.

[dasjestyr]

Basically, I think maybe our desire to mock HttpClient is in a way related to the average developer's propensity to spin up new instances of HttpClient instead of reusing them which is terrible practice. In other words, we underestimate what HttpClient is and should focus on the handler instead. Also, the handler is abstract, so it can be easily mocked.

Ok, I'm sold. I no longer want the interface on HttpClient for mocking.

[ppumkin]

After struggling for way to long I made my own IHttpHandler Interface and implemented everything for my use case. Makes my concrete code easier to read and tests can be mocked out without some creeazy ass stuff.

[jnm2]

Came across this today: http://www.davesquared.net/2011/04/dont-mock-types-you-dont-own.html

[abatishchev]

It's near impossible to share an instance of HttpClient imn any really application as soon as you need to send different HTTP header on each request (what is crucial when communicating with properly designed RESTful web services). Currently HttpRequestHeaders DefaultRequestHeaders is bound to the instance and not to a call on it effectively making it stateful. Instead {Method}Async() should accept it what would make HttpClient stateless and really reusable.

[richardszalay]

@abatishchev But you can specify headers on each HttpRequestMessage.

[abatishchev]

@richardszalay I don't say it's completely impossible, I say that HttpClient wasn't designed well for this purpose. None of {Method}Async() accept HttpRequestMethod, only SendAsync() does. But what's the purpose of the rest then?

[jnm2]

But what's the purpose of the rest then?

Meeting 99% of the needs.

[abatishchev]

Does it mean setting headers is 1% of use cases? I doubt.

Either way this won't be an issue if those methods​ had an overload accepting HttpResponseMessage.

[jnm2]

@abatishchev I don't doubt it, but either way, I'd write extension methods if I found myself in your scenario.

[dasjestyr]

I toyed with the idea of maybe interfacing HttpMessageHandler and possibly HttpRequestMessage because I didn't like having to write fakes (vs. mocks). But the further down that rabbit hole you go, the more you realize that you'll be trying to fake actual data value objects (e.g. HttpContent) which is a futile exercise. So I think that designing your dependent classes to optionally take HttpMessageHandler as a ctor argument and using a fake for unit tests is the most appropriate route. I'd even argue that wrapping HttpClient is also a waste of time...

This will allow you to unit test your dependent class without actually making a call to the internet, which is what you want. And your fake can return pre-determined status codes and content so that you can test that your dependent code is processing them as expected, which is again, what you actually want.

[PureKrome]

@dasjestyr Did you try creating an interface for HttpClient (which is like creating a wrapper for it) instead of interfaces for HttpMessageHandler or HttpRequestMessage ..

/me curious.

[dasjestyr]

@PureKrome I sketched out creating an interface for it and that's where I quickly realized that it was pointless. HttpClient really just abstracts a bunch of stuff that doesn't matter in the context of unit testing, and then calls the message handler (which was a point that was made a few times in this thread). I also tried creating a wrapper for it, and that was simply not worth the work required to implement it or propagate the practice (i.e. "yo, everyone do this instead of using HttpClient directly"). It's MUCH easier to simply focus on the handler, as it gives you everything you need and is literally comprised of a single method.

That said, I have created my own RestClient, but that solved a different problem which was providing a fluid request builder, but even that client accepts a message handler that can be used for unit testing or for implementing custom handlers that handle things like handler chains that solve cross-cutting concerns (e.g. logging, auth, retry logic, etc.) which is the way to go. That's not specific to my rest client, that's just a great use-case for setting the handler. I actually like the HttpClient interface in the Windows namespace much better for this reason, but I digress.

I think it could still be useful to interface the handler, however, but it would have to stop there. Your mocking framework can then be setup to return pre-determined instances of HttpResponseMessage.

[PureKrome]

Interesting. I've found (personal bias?) my helper library works great when using a (fake) concrete message handler .. vs.. some interface stuff on that lowish level.

I would still prefer not to have to write that library or use it, though :)

[dasjestyr]

I don't see any problem with creating a small library to build fakes. I might do so when I'm bored with nothing else to do. All my http stuff is already abstracted and tested so I have no real use for it at the moment. I just don't see any value in wrapping the HttpClient for the purpose of unit testing. Faking the handler is all you really need. Extending functionality is a completely separate topic.

[abatishchev]

When the most of the codebase is tested using mocking interfaces, it's more convenient and consistent when the rest of the codebase is tested the same way. So I would like to see an interface IHttpClient. Like IFileSystemOperarions from ADL SDK or IDataFactoryManagementClient from ADF management SDK, etc.

[dasjestyr]

I still think you're missing the point, which is HttpClient doesn't need to be mocked, only the handler. The real problem is the way that people look at HttpClient. It's not some random class that should be newed up whenever you think to call the internet. In fact, it's best practice to reuse the client across your entire application -- it's that big of a deal. Pull the two things apart, and it makes more sense.

Plus, your dependent classes shouldn't care about the HttpClient, only the data that gets returned by it -- which comes from the handler. Think of it this way: are you ever going to replace the HttpClient implementation with something else? Possible... but not likely. You never need to change the way the client works, so why bother abstracting it? The message handler is the variable. You'd want to change how the responses get handled, but not what the client does. Even the pipeline in WebAPI is focused on the handler (see: delegating handler). The more I say it, the more I start to think that .Net should make the client static and manage it for you... but I mean... whatever.

Remember what interfaces are for. They're not for testing -- it was just a clever way to leverage them. Creating interfaces solely for that purpose is ridiculous. Microsoft gave you what you need to decouple the message handling behavior, and it works perfectly for testing. Actually, HttpMesageHandler is abstract, so I think most mocking frameworks like Moq would still work with it.

[PureKrome]

Heh @dasjestyr - I too think you might have missed a major point of my discussion.

The fact that we (the developer) needs to _learn_ so much about Message Handlers, etc .. to fake the response is my _main_ point about all of this. Not so much about interfaces. Sure, I (personally) prefer interfaces to virtuals r wrappers with respect to testing (and therefore mocking) ... but those are implementation details.

I'm hoping the main gist of this epic thread is to highlight that .. when using HttpClient in an application, it's a PITA to test with it.

The status quo of "go learn the plumbing of HttpClient, which will lead you to HttpMessageHandler, etc" is a poor situation. We don't need to do this for many other libraries, etc.

So I was hoping something can be done to alleviate this PITA.

Yes, the PITA is an opinion - I'll totally admit to that. Some people don't think it's a PITA at all, etc.

The real problem is the way that people look at HttpClient. It's not some random class that should be newed up whenever you think to call the internet.

Agreed! But until fairly recently, this wasn't well known - which might lead to some lack of documentation or teaching or something.

Plus, your dependent classes shouldn't care about the HttpClient, only the data that gets returned by it

Yep - agreed.

which comes from the handler.

a what? huh? Oh -- now u're asking me to open up the hood and learn about the plumbing? ... Refer to above again and again.

Think of it this way: are you ever going to replace the HttpClient implementation with something else?

No.

.. etc ..

Now, I'm not meaning to troll, etc. So please don't think that I'm trying to be a jerk by always replying and repeating the same stuff over and over again.

I've been using my helper library for ages now which is just a glorified way of using a custom message handler. Great! It works and works great. I just think that this could be _all_ exposed a bit nicer ... and that's what I'm hoping this thread really hits home at.

EDIT: formatting.

[richardszalay]

(I've only just noticed the grammar error in this issue's title and now I can't unsee it)

[PureKrome]

And that has been my _real_ long game :)

Q.E.D.

(actually - so embarrassing 😊 )

EDIT: part of me doesn't want to edit it .. for nostalgia....

[jnm2]

(I've only just noticed the grammar error in this issue's title and now I can't unsee it)

I know! Same!

[dasjestyr]

The fact that we (the developer) needs to learn so much about Message Handlers, etc .. to fake the response is my main point about all of this. Not so much about interfaces. Sure, I (personally) prefer interfaces to virtuals r wrappers with respect to testing (and therefore mocking) ... but those are implementation details.

wut?

Have you even looked at HttpMessageHandler? It's an abstract class that is literally a single method that takes an HttpRequestMessage and returns a HttpResponseMessage -- made to intercept behavior separate of all the low level transport junk which is exactly what you'd want in a unit test. To fake it, just implement it. The message that goes in is the one that you sent HttpClient, and the response that comes out is up to you. For example, if I want to know that my code is dealing with a json body correctly, then just have the response return a json body that you expect. If you want to see if it's handling 404 correctly, then have it return a 404. It doesn't get more basic than that. To use the handler, just send it in as a ctor argument for HttpClient. You don't need to pull any wires out or learn the internal workings of anything.

I'm hoping the main gist of this epic thread is to highlight that .. when using HttpClient in an application, it's a PITA to test with it.

And the main gist of what many people have pointed out is that you're doing it wrong (which is why it's a PITA, and I say this directly but respectfully) and demanding that HttpClient be interfaced for testing purposes is akin to creating features to compensate for bugs instead of addressing the root problem which is, in this case, that you're operating at the wrong level.

I think that HttpClient in the windows namespace actually did separate the concept of handler into filter, but it does the exact same thing. I think that handler/filter in that implementation is actually interfaced though which is kinda what I was suggesting earlier.

[PureKrome]

wut?
Have you even looked at HttpMessageHandler?

Initially no, because of this:

var payloadData = await _httpClient.GetStringAsync("https://......");

meaning, the exposed methods on HttpClient are really nice and make things _really_ easy :) Yay! Fast, simple, works, WIN!

Ok - so now lets use it in a test ... and now we need to spend time learning what happens underneath .. which leads us to learning about HttpMessageHandler etc.

Again I keep saying this => this extra learning about the plumbing of HttpClient (i.e. HMH, etc) is a PITA. Once you have _done that learning_ .. yes, I then know how to use it etc ... even though I also don't like to keep using it but need to, in order to mock remote calls to 3rd party endpoints. Sure, that's opinionated. We can only agree to disagree. So please - I know about HMH and how to use it.

And the main gist of what many people have pointed out is that you're doing it wrong

Yep - people disagree with me. No probs. Also - people agree me too. Again, no probs.

and _demanding_ that HttpClient be interfaced for testing purposes is akin to creating features to compensate for bugs instead of addressing the root problem which is, in this case, that you're operating at the wrong level.

Ok - I respectfully disagree with you here (which is ok). Again, different opinions.

But srsly. This thread has gone on for so long. I've really moved on by now. I said my piece, some people say YES! some said NO! Nothing has changed and I've just ... accepted the status quo and rolled with everything.

I'm sorry you just don't accept my train of thought. I'm not a bad person and try not sound rude or mean. This was just me expressing how I felt while developing and how I believe others also felt. That's all.

[dasjestyr]

I'm not at all offended. I originally hopped on this thread with the same opinion that the client should be mockable, but then some really good points were made about what HttpClient is so I sat and really thought about it, and then tried to challenge it on my own and eventually I came to the same conclusion which is that HttpClient is the wrong thing to mock and trying to do so is futile exercise that causes far more work than it's worth. Accepting that made life much easier. So I too have moved on. I'm just hoping that others will eventually give themselves a break and take it for what it is.

[dasjestyr]

As an aside:

Initially no, because of this:

var payloadData = await _httpClient.GetStringAsync("https://......");

meaning, the exposed methods on HttpClient are really nice and make things really easy :) Yay! Fast, >simple, works, WIN!

I'd argue that in terms of SOLID, this interface is inappropriate anyway. IMO client, request, response are 3 different responsibilities. I can appreciate the convenience methods on the client, but it promotes tight coupling by combining requests and responses with the client, but that's personal preference. I have some extensions to HttpResponseMessage that accomplish the same thing but keep the responsibility of reading the response with the response message. In my experience, with large projects, "Simple" is never "Simple" and almost always ends in a BBOM. This however, is a completely different discussion :)

[davidsh]

Now that we have a new repo specifically for design discussions, please continue the discussion in https://github.com/dotnet/designs/issues/9 or open a new issue in https://github.com/dotnet/designs

Thanks.

[AchotStepanian]

Maybe it could be considered below solution for testing purpose only:

public class GeoLocationServiceForTest : GeoLocationService, IGeoLocationService
{
public GeoLocationServiceForTest(something: something, HttpClient httpClient) : base(something)
{
base._httpClient = httpClient;
}
}

[arialdomartini]

I ended up using @richardszalay's MockHttp.
Thanks, Richard, you saved my day!

Ok, I can live with a no-interfaced HttpClient. But being forced to implement a class inheriting HttpMessageHandler because SendAsync is protected just sucks.

I use NSubstitute, and this doesn't work:

var httpMessageHandler = 
    Substitute.For<HttpMessageHandler>(); // cannot be mocked, not virtual nor interfaced
httpMessageHandler.SendAsync(message, cancellationToken)
    .Return(whatever); // doesn't compile, it's protected
var httpClient = new HttpClient(httpMessageHandler)

Really disappointing.