dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.57k stars 25.3k forks source link

total rewrite & container issue: unit test ASP.NET Core apps #15839

Open Rick-Anderson opened 4 years ago

Rick-Anderson commented 4 years ago

This topic:


This is the container issue for this doc. Reopen each issue you are working on so the owner is notified.

PU issue

Not PU:


Total rewrite outline:


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ardalis commented 4 years ago

I’d avoid unit testing controllers entirely. They’re better tested using integration tests, which can verify filters, routes, model binding, model validation, etc.

Rick-Anderson commented 4 years ago

@ardalis there is a place for unit tests. https://github.com/aspnet/AspNetCore/tree/master/src is full of them.

ardalis commented 4 years ago

Of course - they're testing the framework. Folks building apps on the framework should be testing their application logic. I've done it both ways and found that with the great integration/functional testing capabilities of ASP.NET Core you get better tests that run almost as quickly by testing endpoints, not controller methods.

You can unit test controller action methods, but the tests tend to be more brittle and tightly coupled than you might like. And like I said above, the tests won't tell you anything about filters, routes, model validation, model binding, etc. all of which you probably should be testing.

If you want to see an example of how testing endpoints instead of methods allows for refactoring of those methods, have a look at this refactoring:

Start: https://github.com/ardalis/GettingStartedWithFilters/blob/master/Filters101/Controllers/AuthorsController.cs#L52-L66

End: https://github.com/ardalis/GettingStartedWithFilters/blob/master/Filters101/Controllers/Authors2Controller.cs#L44-L51

Integration tests that still pass for both before/after refactoring, which didn't change external behavior (note each test hits both controllers): https://github.com/ardalis/GettingStartedWithFilters/blob/master/IntegrationTests/AuthorsController/Put.cs

If the before code had had unit tests, they would all have broken when the refactoring took place

Rick-Anderson commented 4 years ago

Total rewrite outline:

@anurse please assign someone to review this outline

analogrelay commented 4 years ago

This is a very MVC-centric document currently, would it still be MVC-centric? I can have someone review it, but it might be good to have someone on @mkArtakMSFT 's team look at it since they'd have experience with MVC and unit testing.

Rick-Anderson commented 4 years ago

@anurse no, it will be MVC, Razor Pages, and Web API. @mkArtakMSFT can you assign someone to review my outline?

@ardalis suggests we don't show unit testing at all and just delete this article.

ardalis commented 4 years ago

...or if not delete it, use it to talk about why unit testing controller methods has disadvantages and talk up unit testing other things (like your services and domain logic) but using integration tests for controller actions, etc. :)

This used to live in the asp.net core docs long ago but you could certainly link to it from such an article if you didn't just delete it: https://docs.microsoft.com/en-us/dotnet/core/testing/

analogrelay commented 4 years ago

Honestly, I do tend to lean more towards integration tests these days (using things like TestServer instead of testing ActionResults), but I know that's not a universal opinion and I don't build many apps these days ;). Having some documentation on testing is still quite valid.

Rick-Anderson commented 4 years ago

@javiercn please approve or reject the proposed outline of total rewrite of this article

StingyJack commented 4 years ago

@Rick-Anderson, whats a CSAT ?

I would like to also point out that this has [Fact] and not [TestMethod]. Folks who are coming here to understand how to test a controller may not be experienced enough with unit testing to be using something other than MsTest/TestFx (the thing that comes as part of visual studio).

Trying to throw one of the other test frameworks into this is just adding confusion and requires the reader to learn "yet another framework" just to use the information in this article. This increases the chances that someone will forego this information and ultimately not unit test more of their code.

The TOC is way too short for this much text. A comparison of the ratio of headings to length of text may be a useful check to perform to catch rambling articles like this.

The article needs to exist, because controllers are often where business logic is placed, and that kind of thing needs to be unit tested easily. If something is not easy, people are going to forego it when any pressures are applied.

Rick-Anderson commented 4 years ago

@StingyJack Customer SATisfaction.

Rewrite will be very similar to https://docs.microsoft.com/en-us/dotnet/core/testing/unit-testing-with-dotnet-test

StingyJack commented 4 years ago

Thanks for the clarification. I'm sure it's probably not so simple so I won't ask about the details of what makes low vs high.

I do hope that you take a different path with the article though; CSAT for me (and most of the people I've worked with over the years) is going to be low or non-existent once I have to regress to using the CLI instead of using a more efficient keyboard shortcut. Coupling that with some third party testing framework just increases the speed at which we close the page and look for a different web search result, or just give up and stop looking ("I just wanted to do something good and unit test these controllers, but I have to use - and learn - and tool up the team and any processes for - some other unit test framework? Nevermind.")

mkArtakMSFT commented 4 years ago

@mkArtakMSFT can you assign someone to review my outline?

@Rick-Anderson I will get sombeody to review this within a week.

javiercn commented 4 years ago

I'm a bit lost about why we are trying to change this topic or what are we trying to change here. Can you help me with:

  1. Link to the doc that we want to change.
  2. Brief description of the changes.
  3. Motivation for the change.
Rick-Anderson commented 4 years ago

@javiercn all those questions are answered at the top of the issue

  1. Link to the doc that we want to change.
mkArtakMSFT commented 4 years ago

@Rick-Anderson I've read through the article as well as through the IsPrime-based article in the .Net Core documentation and here what I think about the current articles.

Pros

Cons

I recommend removing the concept of a repository, but having small subsections in the article where the NotFoundResult and RedirectResult are still used. Also, it'll be worth using an example where a service dependency still exist but in more generic way (IPrimeService from the referenced article will be fine).

So here what I think may be a good layout:

Overview

This article shows examples and best practices about how to unit-test controller actions. Both API controller as well as MVC View controller actions will be demonstrated in this article.

Prerequisites

To make sure you get most out of this article it'll be best to have some understanding about what unit-tests are as well as what are different unit-test frameworks. We're going to use xUnit (or whatever is going to be used here) framework for unit-testing, but other test frameworks can be used too. See the [Other unit-test frameworks]() section to learn how.

Unit-testing API controllers

Setting up the test project

Testing controllers which return specific results

public bool IsPrime(int number)
{
   // this is just an example for demonstration, feel free to change it as necessary
}

// or

public Task<bool> IsPrime(int number)
{

}

Testing controllers which return no results

There are situations, where a controller action doesn't actually return any results. Instead, the effect of an action is somehow reflected / applied on an underlying service the controller depends on.

public void StoreNumber (int number)
{
}

// or

public Task StoreNumber(int number)
{
}

Testing controllers which return IActionResult or ActionResult<T>

public ActionResult<bool> IsPrime(int number)
{
}

In this section bring examples of testing NotFoundResult or RedirectionResult results.

Other unit-test frameworks

In this section we should bring examples of how to use few other popular unit-test frameworks like xUnit (if this wasn't the framework above), NUnit and maybe one more framework. We should have sub-sections per each one and have a very small sample and reference to their docs websites.

@Rick-Anderson thoughts?

Rick-Anderson commented 4 years ago

@mkArtakMSFT sounds great

StingyJack commented 4 years ago

@StingyJack Customer SATisfaction.

Rewrite will be very similar to https://docs.microsoft.com/en-us/dotnet/core/testing/unit-testing-with-dotnet-test

This response should be marked as "off-topic" as well as most of the responses to the OP, if asking questions about the words used in the topic and giving feedback specific to the goal described in the OP is considered "off-topic".