Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to mock requests from Protected methods in sequences #800

Open
bensmith009988 opened this issue Apr 29, 2024 · 1 comment
Open
Labels
feature-request Request for a new NSubstitute feature

Comments

@bensmith009988
Copy link

I fall into a category of people that has transitioned to using NSubstitute after discovering security concerns in Moq, which are detailed thoroughly in this article: https://medium.com/@michalsitek/critical-security-vulnerability-in-moq-4-20-0-ffd24739cc49.

So far, there has been only 1 feature of NSubstitute that was present in Moq, that is no longer possible: Mocking protected methods easily. Our company often writes unit tests for classes that make outbound http requests. In these classes that make the requests, we typically access the System.Net.Http.HttpClient via constructor injection. Mocking Http Requests is not exactly trivial with any mocking library, and usually involves some form of mocking the actual HttpMessageHandler, and building a new client with the mocked handler.

In Moq, you could skip all of this and simply mock the response of the protected method SendAsync in the unit test setup, by passing in a string value of the name of the protected method, as well as the request parameters to the method the mocked response is expecting. (Mocking the actual REST methods on the HttpClient doesn't actually work, because they all eventually call "SendAsync", which is the protected method we need to override). Here is an example of how simple it was to mock an Http request (regardless if it was Post, Put, Delete, or Get) looked like using Moq, because of the feature described:

var handlerMock = new Mock<HttpMessageHandler>();
            handlerMock.Protected()
                       .Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
                       .ReturnsAsync(new HttpResponseMessage());

This was great because it offered a way to get granular with exactly what each Request message looked like, (by identifying requests from their outbound target address), and provide specific responses for each request. You could call "SetupSequence," and string together all of your specific requests and each specific response.

Because NSubsitute doesn't have a way to override protected methods like this, we are forced to mock the HttpClientFactory
CreateClient() method. To do this, we have a FakeHttpMessageHandler class that is used on top of the DelegatingHandler to override the protected method "SendAsync." While technically this is a workaround, and does work for a single outbound http request, it only works for 1 outbound http request. We can't mock multiple requests/responses to the SendAsync method. This matters because on methods where we have 3 and 4 outbound requests with specific logic that changes the cyclical complexity and logic routing, we lose the ability to write a whole suite of thorough test cases. We also can't specify WHICH specific incoming request should have WHICH specific response. We just know that eventually SendAsync will be called, and when it is, toss it this response.

You can see our FakeHttpMessageHanlder workaround below, and you will understand what I mean. We have attempted to come up with ways to inject multiple response, and sequencing, but have been unsuccessful at doing so. Additionally, we are having to include this workaround in every project we write tests for. We have over 130 repositories with LOTS of testing projects within. It is becoming cumbersome to maintain a workaround in tons of places. Please introduce this feature. It would save a lot of headache from your users. I know we are not alone, as the web is riddled with workarounds to this same problem. I have personally not seen a single implementation of a workaround that allows for getting granular on mocking sequences of requests/responses.

The FakeHttpMessageHandler that overrides the SendAsync method on the original DelegatingHandler:

public class FakeHttpMessageHandler : DelegatingHandler
{
    private HttpResponseMessage fakeResponse;

    public FakeHttpMessageHandler(HttpResponseMessage responseMessage)
    {
        fakeResponse = responseMessage;
    }

    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        return await Task.FromResult(fakeResponse);
    }
}

An example of how we use the FakeHttpMessageHandler, referenced in each of our tests that needs to mock a response to an outbound HttpRequest.

[Fact]
public async Task SomeMethod_ReturnsAPIResponse()
{
    //Arrange
    var expectedResponse = fixture.Create<SomeObject>();

    clientFactory.CreateClient(Arg.Any<string>())
                         .Returns(new HttpClient(SetupMessageHandler(HttpStatusCode.OK, expectedResponse)));

    UnitTestBuilder builder = new UnitTestBuilder()
        .Test<WebApiService>()
        .With(clientFactory)
        .Build();

    var service = builder.Services.GetRequiredService<WebApiService>();

    //Act
    var result = await service.SomeMethodWithArgs(fixture.Create<string>());

    //Assert
    result.Should().BeEquivalentTo(expectedResponse );
}

private static FakeHttpMessageHandler SetupMessageHandler(HttpStatusCode returnStatusCode, object returnObject)
{
    return new FakeHttpMessageHandler(new HttpResponseMessage
    {
        StatusCode = returnStatusCode,
        Content = new StringContent(JsonConvert.SerializeObject(returnObject), Encoding.UTF8, "application/json")
    });
}

Hopefully you are able to see the downsides in test cases without the ability to do what Moq provided. Hopefully the verbiage on this post made sense, but I am happy to clarify.

@304NotModified 304NotModified added the feature-request Request for a new NSubstitute feature label Apr 29, 2024
@304NotModified
Copy link
Contributor

Related #222 (with a working extension method? )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for a new NSubstitute feature
Projects
None yet
Development

No branches or pull requests

2 participants