Skip to content

Min api unit & integration tests sample#26801

Merged
Rick-Anderson merged 18 commits into
dotnet:mainfrom
fiyazbinhasan:min-api-unit-test
Sep 19, 2022
Merged

Min api unit & integration tests sample#26801
Rick-Anderson merged 18 commits into
dotnet:mainfrom
fiyazbinhasan:min-api-unit-test

Conversation

@fiyazbinhasan
Copy link
Copy Markdown
Contributor

@fiyazbinhasan fiyazbinhasan commented Aug 22, 2022

contributes to #26352

@Rick-Anderson In my opinion, this could make a stand-alone comprehensive doc about writing unit and integration tests for minimal API apps. Please take a look at the sample. I've modified the existing todo-group sample to support the test cases. The sample could also be used to showcase the MapGroup,

https://devblogs.microsoft.com/dotnet/asp-net-core-updates-in-dotnet-7-preview-4/#route-groups

@Rick-Anderson
Copy link
Copy Markdown
Contributor

@captainsafia please assign a reviewer

@Rick-Anderson Rick-Anderson mentioned this pull request Aug 25, 2022
3 tasks
Copy link
Copy Markdown
Contributor

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brunolins16 Do you want to give this a check?

@@ -0,0 +1,135 @@
using Microsoft.AspNetCore.Http.HttpResults;
using todo_group;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: We probably want to use PascalCase naming for this namespace.

@Rick-Anderson Rick-Anderson self-assigned this Sep 7, 2022

namespace todo_group;

public static class TodoEndpointsV2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the goal in add a V2?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V1 and V2 follow different patterns for accessing the underlying DB context. In V1 the context is directly injected into the endpoint whereas in V2 an additional abstraction (TodoService) is used to wrap the DB context. I think this is very common to use this abstraction (e.g. Repository pattern) and adopted in many applications to make the endpoint/controllers more testable. Rather than faking the context with an In-Memory implementation, we use a mock of a service. To show this, I've used Moq for mocking the TodoService in TodoMoqTests. Similarly for V1, I've used the In-Memory database approach found in TodoInMemoryTests

@@ -6,11 +6,19 @@
<ImplicitUsings>enable</ImplicitUsings>
<RootNamespace>todo_group</RootNamespace>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename the csproj to TodoGroup.csproj?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should update the namespace.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, agreed. Will do. @Rick-Anderson could you please verify that this sample has no additional references in the documentation? This was an existing sample that I found and modified. I'm not sure whether it will break anything somewhere

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Nope

Comment on lines +90 to +91
var createdResult = (Created<Todo>)await TodoEndpointsV1.UpdateTodo(updatedTodo, context);
var notFoundResult = (NotFound)await TodoEndpointsV1.UpdateTodo(new Todo { Id = 2, Title = "Invalid Title" }, context);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about splitting the tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for documented examples and as a unit, this is ok. If I separate these out there will be three more test methods.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 tests would be better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added another fact just to check whether the todo is available inside the db,

TodoInMemoryTests.cs (When DbContext is injected)

[Fact]
public async Task GetTodoReturnsNotFoundIfNotExists()
{
    // Arrange
    await using var context = new MockDb().CreateDbContext();

    // Act
    var notFoundResult = (NotFound)await TodoEndpointsV1.GetTodo(404, context);

    //Assert
    Assert.Equal(404, notFoundResult.StatusCode);
}

TodoMoqTests.cs (When a service wrapper of the DbContext is injected)

[Fact]
public async Task GetTodoReturnsNotFoundIfNotExists()
{
    // Arrange
    var mock = new Mock<ITodoService>(); 
    
    mock.Setup(m => m.Find(It.Is<int>(id => id == 404)))
        .ReturnsAsync((Todo?)null);

    // Act
    var notFoundResult = (NotFound)await TodoEndpointsV2.GetTodo(404, mock.Object);

    //Assert
    Assert.Equal(404, notFoundResult.StatusCode);
}

Comment on lines +45 to +49
.AddEndpointFilter(async (context, next) =>
{
app.Logger.LogInformation("Accessing todo endpoints");
return await next(context);
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we planning showcase filters here as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can exclude filters here

Comment thread aspnetcore/fundamentals/minimal-apis/7.0-samples/todo-group/Program.cs Outdated
Copy link
Copy Markdown
Contributor Author

@fiyazbinhasan fiyazbinhasan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brunolins16 could you please review my comments

Comment thread aspnetcore/fundamentals/minimal-apis/7.0-samples/todo-group/Program.cs Outdated

namespace todo_group;

public static class TodoEndpointsV2
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V1 and V2 follow different patterns for accessing the underlying DB context. In V1 the context is directly injected into the endpoint whereas in V2 an additional abstraction (TodoService) is used to wrap the DB context. I think this is very common to use this abstraction (e.g. Repository pattern) and adopted in many applications to make the endpoint/controllers more testable. Rather than faking the context with an In-Memory implementation, we use a mock of a service. To show this, I've used Moq for mocking the TodoService in TodoMoqTests. Similarly for V1, I've used the In-Memory database approach found in TodoInMemoryTests

@@ -6,11 +6,19 @@
<ImplicitUsings>enable</ImplicitUsings>
<RootNamespace>todo_group</RootNamespace>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, agreed. Will do. @Rick-Anderson could you please verify that this sample has no additional references in the documentation? This was an existing sample that I found and modified. I'm not sure whether it will break anything somewhere

Comment on lines +45 to +49
.AddEndpointFilter(async (context, next) =>
{
app.Logger.LogInformation("Accessing todo endpoints");
return await next(context);
});
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can exclude filters here

Comment on lines +90 to +91
var createdResult = (Created<Todo>)await TodoEndpointsV1.UpdateTodo(updatedTodo, context);
var notFoundResult = (NotFound)await TodoEndpointsV1.UpdateTodo(new Todo { Id = 2, Title = "Invalid Title" }, context);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for documented examples and as a unit, this is ok. If I separate these out there will be three more test methods.

fiyazbinhasan and others added 3 commits September 14, 2022 12:30
…ntegrationTests/TodoIntegrationTestsV1.cs

Co-authored-by: Bruno Oliveira <brunolins16@users.noreply.github.com>
…ntegrationTests/TodoIntegrationTestsV2.cs

Co-authored-by: Bruno Oliveira <brunolins16@users.noreply.github.com>
@fiyazbinhasan fiyazbinhasan requested review from Rick-Anderson and brunolins16 and removed request for Rick-Anderson September 16, 2022 04:34
<PackageReference Include="Microsoft.EntityFrameworkCore.Sqlite" Version="6.0.7" />
<PackageReference Include="Swashbuckle.AspNetCore" Version="6.2.3" />
<PackageReference Include="Microsoft.AspNetCore.OpenApi" Version="7.0.0-preview.7.22376.6" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="7.0.0-preview.7.22376.2">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update and test with V 7 RC1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

packages are updated to RC1. Please check WebMinRouteGroup.csproj as the project namespace has been updated

@Rick-Anderson
Copy link
Copy Markdown
Contributor

@fiyazbinhasan see gitignore
Don't we want the sln file for test projects?
sln files are ignored

@Rick-Anderson
Copy link
Copy Markdown
Contributor

@brunolins16 this looks good so I'm going to merge once the .sln file is pushed - unless you have any changes needed.

@fiyazbinhasan
Copy link
Copy Markdown
Contributor Author

@fiyazbinhasan see gitignore Don't we want the sln file for test projects? sln files are ignored

@Rick-Anderson moved both projects under MinApiTestsSample folder and added the solution file

@Rick-Anderson Rick-Anderson merged commit 619e39c into dotnet:main Sep 19, 2022
@Rick-Anderson
Copy link
Copy Markdown
Contributor

Nice work @fiyazbinhasan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants