Make IResult methods more testable #37502
Description
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