Make IResult methods more testable

[halter73]

Is your feature request related to a problem? Please describe.

When you are defining minimal route handlers using named methods instead of lambdas for better unit testability the inability to inspect the state of the IResults returned by the methods in Results makes unit testing far more difficult.

Endpoint definition

public static class TodoEndpoints
{
    // Program.cs or whatever configures the IEndpointRouteBuilder can define the endpoint as follows:
    // app.MapGet("/todos/{id}", TodoEndpoints.GetTodo);
    public static async Task<IResult> GetTodo(TodoDb db, int id)
    {
        return await db.Todos.FirstOrDefaultAsync(todo => todo.Id == id) is Todo todo
            ? Results.Ok(todo)
            : Results.NotFound();
    }
}

Broken Test Code

While an endpoint like this can be tested using WebApplicationFactory as described here, this is more of a end-to-end test than a unit test as this also tests host initialization and the entire middleware pipeline. While it's possible to replace services with mocks this way, it's far more involved than just calling GetTodo(TodoDb db, int id) in a test directly as follows.

[Fact]
public async Task GetTodoReturnsTodoFromDatabase()
{
    var todo = new Todo { Id = 42, Name = "Improve Results testability!" };
    var mockDb = new MockTodoDb(new[] { todo });

    // The next line throws an InvalidCastException because the Microsoft.AspNetCore.Http.Result.OkObjectResult
    // returned by Results.Ok(todo) is internal unlike Microsoft.AspNetCore.Mvc.OkObjectResult.
    var result = (OkObjectResult)await TodoEndpoints.GetTodo(mockDb, todo.Id);

    Assert.Equal(200, result.StatusCode);
    Assert.Same(todo, result.Value);
}

This test will result in an InvalidCastException because the Microsoft.AspNetCore.Http.Result.OkObjectResult returned by Results.Ok(todo) is internal unlike Microsoft.AspNetCore.Mvc.OkObjectResult.

Working Test Code

It is possible to write a similar test by executing IResult.ExecuteAsync(HttpContext) with a mock HttpContext, but it is far more involved.

private static HttpContext CreateMockHttpContext() =>
    new DefaultHttpContext
    {
        // RequestServices needs to be set so the IResult implementation can log.
        RequestServices = new ServiceCollection().AddLogging().BuildServiceProvider(),
        Response =
        {
            // The default response body is Stream.Null which throws away anything that is written to it.
            Body = new MemoryStream(),
        },
    };

[Fact]
public async Task GetTodoReturnsTodoFromDatabase()
{
    var todo = new Todo { Id = 42, Name = "Improve Results testability!" };
    var mockDb = new MockTodoDb(new[] { todo });
    var mockHttpContext = CreateMockHttpContext();

    var result = await TodoEndpoints.GetTodo(mockDb, todo.Id);
    await result.ExecuteAsync(mockHttpContext);

    // Reset MemoryStream to start so we can read the response.
    mockHttpContext.Response.Body.Position = 0;
    var jsonOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web);
    var responseTodo = await JsonSerializer.DeserializeAsync<Todo>(mockHttpContext.Response.Body, jsonOptions);

    Assert.Equal(200, mockHttpContext.Response.StatusCode);
    Assert.Equal(todo, responseTodo);
}

Describe the solution you'd like

We should consider making the IResult implementations in the Microsoft.AspNetCore.Http.Result public so the IResults returned by the Results methods can be casted to something that can be inspected by unit tests similar to the MVC ActionResult types.

However, we should consider how this might create more confusion for developers using both MVC and minimal route handlers about which result types should be used when. For example, having multiple public OkObjectResult implementations in different namespaces could make it hard to determine which OkObjectResult to use when. Different names might mitigate the confusion but I doubt it would eliminate it. This kind of concern is why we made MVC From*Attributes like [FromServices] work with minimal route handler parameters instead of introducing new parameter attributes.

It also might be unclear if you should call the constructor of the newly-public result types or use the existing IResult methods. We could keep the constructors internal, but that might be unnecessarily restrictive.

Since minimal route handlers do not support content negotiation, having separate public ObjectResult and JsonResult types might be unnecessary.

Additional context

Thank you @Elfocrash for demonstrating this issue in your video at https://www.youtube.com/watch?v=VuFQtyRmS0E

[julealgon]

This is probably not the most appropriate place to ask this question, but the repo doesn't have discussions enabled.

Could anyone elaborate why IResult was created in the first place, and why we now have 2 distinct types for returning results from APIs? Wouldn't it be possible to unify these in a single type used by both "standard" actions and minimal actions? Is it related to generics limitations in C# in combination with lambdas? Could someone link some document explaining the thought process behind this new interface and response classes?

[davidfowl]

IActionResult - Tied to ActionContext (requires another allocation per request and is tied to other things like an ActionDescriptor)

I don't think we can unify these types but we can make IResult work in both places.

[halter73]

I don't think we can unify these types but we can make IResult work in both places.

We already did that for many of the existing ActionResult's before merging #33843. We got pretty far with community contributions supporting most of the existing ones (#32565). However, some of the most popular ones like ObjectResult were never ported because they would have to behave differently (e.g. Always produce JSON rather than do content negotiation). Ultimately, we decided it was too confusing to just have a subset of ActionResults work with minimal route handlers but not all of them.

[julealgon]

IActionResult - Tied to ActionContext (requires another allocation per request and is tied to other things like an ActionDescriptor)

I don't think we can unify these types but we can make IResult work in both places.

Thanks @davidfowl . I guess this naturally leads to my next question: what prevented ActionDescription to be also used for minimal APIs as well? I don't want to drag this topic so if there is some reference out there detailing this new response type/interface approach and the hard split between the "MVC" and "HTTP" namespaces that would be really nice. I thought I followed the development on NET6 fairly closely, but I had not heard of this split before seeing the docs on MSDN. I assume I probably missed some key article somewhere.

At first glance, it feels incredibly weird to see IResult and IActionResult (and ActionResult<T>) used for basically the same underlying purpose but with mostly incompatible implementations. I'd imagine this is poor for discoverability and API orthogonality in general.

[davidfowl]

Thanks @davidfowl . I guess this naturally leads to my next question: what prevented ActionDescription to be also used for minimal APIs as well?

We wanted something lower level that doesn't pull in ActionDescriptors and doesn't rely on any of the MVC namespaces. We rebuilt MVC on top of endpoint routing in .NET Core 3.1 and this is a continuation of moving primitives out of MVC and into the core platform. There will be some inconveniences for sure (like the ones this issue describes) but those are typical growing pains because MVC was first. This is the reality when trying to retrofit an existing universe on top of a new core. We don't have it written down publicly but there is a north start where MVC is an extensible framework built on top of endpoint routing and route metadata. When it was created, MVC got all of the goodies (routing, model binding, action results, filters) and now we're slowly peeling back those features into the core frameworks so other parts of the stack can benefit from them

[julealgon]

Very interesting @davidfowl . I'd love to read more about these long-term goals for MVC someday, hopefully a blog post will be considered.

I noticed that attempting to use IResult and its implementations on a normal controller doesn't seem to work properly: the framework doesn't recognize the type/interface, emitting it as if it was a normal object wrapped in a OkObjectResult model. Is this something the team intends to support moving forward as a way to reuse the more basic types, or will standard controllers never support these?

I'd also imagine these simpler models won't work with abstractions such as IResultFilter, which is tied to IActionResult.

The new types seems much more lightweight as you suggested, so I figured there would be a plan for migrating towards them, but this doesn't appear to be the case, at least not currently.

[halter73]

I noticed that attempting to use IResult and its implementations on a normal controller doesn't seem to work properly: the framework doesn't recognize the type/interface, emitting it as if it was a normal object wrapped in a OkObjectResult model. Is this something the team intends to support moving forward as a way to reuse the more basic types, or will standard controllers never support these?

This is something we plan to support. It's currently being tracked by #33403

[brunolins16]

Fixed by #40704