NUnit - testy jednostkowe w .NET Core

Cześć,

Mam pytanie do osób, które znają temat testów jednostkowych. Z tego co na szybko się zorientowałem, takie testy wprowadza się głównie dla prostych funkcjonalności, nie stosuje się ich jeżeli mamy do czynienia z połączeniem z BD (DbContext).

W projekcie, który prowadzę wspólnie ze znajomymi architektura aplikacji na tą chwilę wygląda tak, że w kontrolerach mamy głównie metody GET i POST. Get wyświetla nam odpowiedni widok, natomiast metoda POST wysyła zapytanie do bazy danych. Nie ma obecnie w tej chwili w tym projekcie żadnych "prostych" metod typu: jeśli warunek jest spełniony to true, a jeśli nie to false.

Proszę o informację w jaki sposób (i czy w ogóle jest to możliwe) napisać testy jednostkowe do tego typu metod. Poniżej kilka przykładowych:

1. Logowanie

        [HttpPost]
        public async Task<IActionResult> Login(LoginViewModel model)
        {
            //login functionality
            var user = await _userManager.FindByNameAsync(model.Email);

            if (user != null)
            {
                //sign in
                var signInResult = await _signInManager.PasswordSignInAsync(user, model.Password, false, false);
                if (signInResult.Succeeded)
                {
                    return RedirectToAction("Index", "Home");
                }
            }

            return RedirectToAction("Index", "Home");
        }

1. Weryfikacja eMail

        public async Task<IActionResult> VerifyEmail(string userId, string code)
        {
            var user = await _userManager.FindByIdAsync(userId);

            if (user == null) return BadRequest();

            var result = await _userManager.ConfirmEmailAsync(user, code);

            if (result.Succeeded)
            {
                return View();
            }

            return BadRequest();
        }

1. Wylogowanie

        public async Task<IActionResult> LogOut()
        {
            await _signInManager.SignOutAsync();
            return RedirectToAction("Index", "Home");
        }

Jak dla mnie to za dużo masz w tych Controllerach. Ja bym to wyniósł do osobnej klasy i testował te klasę właśnie.
Tu masz w dokumentacji przykładowe testowanie: https://docs.microsoft.com/pl-pl/aspnet/core/mvc/controllers/testing?view=aspnetcore-3.1

Ale chyba najlepiej było by testować integracyjnie coś w tym stylu: https://docs.microsoft.com/pl-pl/aspnet/core/test/integration-tests?view=aspnetcore-3.1

@szydlak:

Tutaj według mnie warto zaznaczyć że lepiej trzymać się z dala potworka zwanego UseInMemoryDatabase

Krispekowy napisał(a):

Mam pytanie do osób, które znają temat testów jednostkowych. Z tego co na szybko się zorientowałem, takie testy wprowadza się głównie dla prostych funkcjonalności, nie stosuje się ich jeżeli mamy do czynienia z połączeniem z BD (DbContext).

Gdzie się zorientowałeś, że takie testy wprowadza się do prostych funkcjonalności? Testy jednostkowe mają testować jedną funkcjonalność i jak najbardziej może ona być skomplikowana. Co do DbContextu to proponuje przeczytać o mockowaniu baz danych w testach jednostkowych np. EF Effort.

A więc podsumowując? Bo widzę że wśród komentujących też są rozbieżności. W takim razie testować czy nie testować? :)

Jeżeli mockowanie może powodować przekłamanie bo nie odzwierciedla relacji to jest to w naszym projekcie rozwiązanie do wyrzucenia.

Korzystamy z EF i wersji .Net core 3.1

Zdecydujcie jako zespół (product owner, developerzy, menadżer). Możesz używać in memory database (chwile mi zeszło z odpowiednim przygotowaniem, ale później działało spoko), możesz testować na specjalnej bazie stworzonej do testów i po każdym teście usuwać dodane rekordy (albo i nie, jeśli to nie problem), możesz robić testy tylko tam gdzie nie jest potrzebne połączenie z bazą. Możliwości jest sporo, ja jednak pisałbym testy.

Na pewno testować, ale zastanów się w jaki sposób chcesz to zrobić

Tutaj ładnie są zebrane wady/zalety prawdziwej bazy, sqlite i InMemory.

Testing code that uses EF Core

Jest też możliwość zrobienia tak, że np. lokalnie testy lecą przez SQLite, a ci/cd używa prawdziwej bazy

Więc masz ten szybki development i łatwość postawienia lokalnie appki oraz rzetelność, bo nadal ci/cd sprawdzi na prawdziwej bazce.

Testy jednostkowe mają testować jednostki, a nie jakieś "funkcjonalności" (czymkolwiek są, bo jedna aplikacja może mieć jedną funkcjonalność, więc stosowanie tu liczby mnogiej odbiera sens wypowiedzi). Celem przetestowania jednostki jest na ogół stwierdzenie, czy dla podanych danych otrzymujesz spodziewane wyniki.
No i teraz jeśli założymy, że jednostką, którą chcesz przetestować jest metoda Login, która zawsze zwraca ten sam RedirectToAction, to właściwie nie ma znaczenia jaki LoginViewModel tam podamy, więc test niczego nie sprawdzi. Podobnie jest z Logout, a z VerifyEmail jest niewiele lepiej.

Ogólnie testowanie jednostkowe kontrolerów nie ma sensu, po pierwsze dlatego, że kontrolery raczej powinny oddelegowywać wykonanie zadania gdzieś dalej niż same zawierać logikę, a po drugie dlatego, że takie testy rzadko będą w jakikolwiek sposób wartościowe i pozwolą wykryć błędy. Co z tego, że test jednostkowy kontrolera przejdzie, skoro aplikacja nie będzie działać, bo nie będzie widoku, do którego chcesz zrobić redirect? Albo żądanie nawet nie osiągnie kontrolera, bo jakiś filtr/middleware/model binder po drodze będzie zepsuty?
W takich przypadkach lepiej testować e2e i faktycznie weryfikować, czy po podaniu prawidłowych danych, użytkownik staje się zalogowany. Interakcja z GUI, to jest coś, co się testuje na postawionej gdzieś aplikacji, często ręcznie, albo przy użyciu specjalizowanych frameworków.

Używaj testów jednostkowych tam, gdzie mają sens - do przetwarzania danych, a nie interakcji użytkownika ze stroną na podstawie danych zawartych w bazie. Np. kod odpowiadający za dodanie dwóch liczb, obliczenie wartości rabatu na podstawie liczby produktów w koszyku oraz dotychczasowej wartości zamówień klienta, obliczający wartość zaliczki na podatek dochodowy na podstawie kwoty zarobków, to są miejsca, w których testy jednostkowe się sprawdzą doskonale. Masz wejście, funkcję i wyjście, sam kod, bez żadnej komunikacji po HTTP, używania baz danych i interakcji z GUI.

Być może lekko uzupełnię wypowiedź somekind.
Nie testuj frameworka ani cudzych bibliotek. Często ludzie wpadają w taką pułapkę, że zaczynają to robić. Np. sprawdzają, czy podając niepoprawne hasło do bazy, połączymy się z nią, czy wyskoczy błąd. To już zostało przetestowane przez twórców tych bibliotek i frameworków. Zatem...

Testuj TYLKO swój kod.
W kontrolerach masz za dużo. Kontrolery powinny być głupie i przekazywać pracę gdzie indziej. Przykładowo u Ciebie - powinieneś pozbyć się z kontrolera obiektu userManager. Za to powinieneś stworzyć sobie taki interfejs:

public interface IUserService
{
    Task<bool> Login(LoginData data);
    Task<bool> Logout();
    Task<bool> VerifyEmail(EmailData data);
}

Następnie powinieneś stworzyć sobie na tej podstawie serwis:

public class UserService: IUserService
{
 //
}

i tutaj mieć całą tą logikę, którą masz w kontrolerze teraz. Razem z obiektem userManager.
Oczywiście musisz pamiętać o tym, żeby ten serwis zarejestrować w dependency injection.

I potem w swoim kontrolerze posługujesz się tylko tym serwisem:

[HttpPost]
        public async Task<IActionResult> Login(LoginViewModel model)
        {
            //login functionality
           bool loginResult = await userService.Login(model); 
           if(loginResult)
             return RedirectToAction("Index", "Home");
           else
             return RedirectToAction("BadLogin", "Home");
        }

I analogicznie z VerifyMail.
Co Ci to daje? Wiele. Przede wszystkim masz głupie kontrolery, które przekazują zadanie dalej. Kontrolery są ściśle powiązane z frameworkiem. Po drugie możesz mockować UserService. Po trzecie - możesz testować UserService. W tym KONKRETNYM przypadku testowanie UserService nie ma jednak sensu, bo skończyłoby się to na testowaniu frameworka. Jeśli Twój UserService się rozrośnie, to wtedy tak. W tym momencie wszystko tak naprawdę delegujesz do UserManagera.
Testuj swój serwis w momencie, gdy zyska on jakąś logikę. Np. po poprawnym zalogowaniu musi coś zrobić. Albo po stworzeniu konta musi utworzyć jakieś dane, stworzyć token do weryfikacji maila i być może nawet wysłać maila aktywacyjnego. I wtedy to testujesz w taki sposób:

• podajesz poprawne dane i patrzysz, czy serwis wysłał maila
• podajesz niepoprawne dane i patrzysz, co się stało (serwis nie powinien wysłać maila)

Co do mockowania - mockowanie to takie trochę oszukiwanie. Popatrz na taki przykład:

public class AccountService: IAccountService
{
    IUserService us;

    public AccountService(IUserService us)
    {
       this.us = us;
    }
}

Jak widzisz mamy tutaj klasę AccountService, którą będziesz chciał testować. Jest ona zależna od UserService. I dlatego stworzyłeś interfejs IUserService, żeby nie posługiwać się konkretną implementacją. Dzięki temu możesz stworzyć nową klasę:

public class FakeUserService: IUserService
{
    public bool ShouldLogin {get; set;}

    public Task<bool> Login(LoginData data)
    {
       return Task.FromResult(ShouldLogin);
    }
}

Stworzyłeś zupełnie nową implementację interfejsu IUserService. Teraz tą klasę możesz przekazać do AccountService podczas testów. A dlaczego tak? Bo testy jednostkowe mają testować jedną konkretną rzecz. Czyli jakąś metodę w klasie AccountService. Jeśli ta metoda jest zależna od UserService, to tak naprawdę testowałbyś jednocześnie AccountService i UserService. W tym przypadku dokładnie wiesz jak się zachowa FakeUserService. I możesz przetestować klasę AccountService jeszcze lepiej. Możesz zobaczyć co się stanie, gdy logowanie się udało, a co, gdy nie. Ale jest coś lepszego. Nie musisz tworzyć klasy FakeUserService, żeby to zrobić. Poczytaj o mockowaniu (i bibliotece Moq). Ona to zrobi za Ciebie w bardzo prosty sposób, np:

[Test]
public void Blabla_UserServiceBadLogin_ShouldThrow()
{
    Mock<IUserService> mock = new Mock<IUserService>();
    mock.Setup(x => x.Login(It.IsAny<LoginData>).ReturnsAsync(false));  //*
   
    AccountService service = new AccountService(mock.Object);
    //i tutaj testujesz metodę w account service
}

• nie pamiętam dokładnie konstrukcji tego mocka, więc coś może się nie zgadzać, ale mniej więcej tak się to robi.

Mam nadzieję, że wyczerpałem temat :)

Gdzie mogę znaleźć informację na temat właśnie tego rodzaju "dobrych praktyk"? Może coś przeoczyłem, ale na stronie Microsoftu też w controllerach zawarta jest logika: https://docs.microsoft.com/pl-pl/aspnet/core/mvc/controllers/testing?view=aspnetcore-3.1

EDIT:
Przejrzałem niektóre opensource'owe projekty na github w tej technologii i zauważyłem, że też w kontrolerach jest sporo logiki. Być może wszyscy popełniają te same błędy, dlatego chciałbym nabyć dobre praktyki jak najwcześniej.

Ten interfejs do UserService i mockowanie tego nie ma sensu, bo nie ma sensu testować kontrolerów.

@Krispekowy: co do kodu w tutorialach, to miej świadomość, że tutorial to nie jest źródło wiedzy o architekturze. A poza tym: http://commitandrun.pl/2016/05/30/Brutalne_prawdy_o_MVC/

@somekind: właśnie o to pytam :) jeżeli nie strona Microsoft jest wyrocznią, jeżeli nie projekty na github innych deweloperów to w takim razie co?

Swoją drogą korzystamy od początku z net core bo jest najbardziej aktualny. MVC chyba ma już kilka lat.

Bez znaczenia, czy masz to MVC w core czy starym frameworku, zasady BHP są takie same.

Wyrocznią jest artykuł, który podlinkowałem.

Krispekowy napisał(a):

@somekind: właśnie o to pytam :) jeżeli nie strona Microsoft jest wyrocznią, jeżeli nie projekty na github innych deweloperów to w takim razie co?

Swoją drogą korzystamy od początku z net core bo jest najbardziej aktualny. MVC chyba ma już kilka lat.

Hmm... zdrowy rozsądek? Kod powinien być tak napisany, aby był utrzymywalny :)

Co do samych testów - ja robię jednostkowe (gdzie np. DbContext jest mockowany, oraz inne jeśli są) oraz integracyjne (tutaj DbContext z testowym SQL lokalnie najczęściej, reszta bez zmian). Jednostkowe testują poszczególne funkcjonalności, które zamknięte są w klasy schowane za interfejsem (a które to są używane przez endpointy w kontrolerach WebAPI). Z kolei integracyjne, to po prostu test HttpClienta, robię requesty do endpointów i sprawdzam co otrzymuje, wtedy wiem, że wszystkie zależności działają razem z bazą i mam prawidłowe odpowiedzi API.

Generalnie, kontrolery powinny odpowiadać tylko za requesty/response'y i walidację itd. Wciskanie tam logiki to imho średni pomysł :)

Jak pisze coś czego nie chce mi się szczegółowo testowac, tylko czy funkcjonalność działa to robie test na kontrolerze. Ale to tylko we własnych projektach, bo czemu by nie, zamiast pisania kilkunastu metod do kazdej klasy mam poglądowo czy działa czy nie.

A co do contextu, jak tam mam powiązane dane i nie chce mi sie ich mockowac czy in memory to tez to robie.
W przypadku jak controller wymaga contextu:

 var optionBuiler = new DbContextOptionsBuilder<ApiDbContext>();
            optionBuiler.UseSqlServer( ConfigurationManager.ConnectionStrings["DefaultConnection"].ConnectionString);

            var context = new ApiDbContext(optionBuiler.Options);
            var controller= new UsersWalletController(context );

Juhas napisał(a):

W kontrolerach masz za dużo. Kontrolery powinny być głupie i przekazywać pracę gdzie indziej. Przykładowo u Ciebie - powinieneś pozbyć się z kontrolera obiektu userManager. Za to powinieneś stworzyć sobie taki interfejs:

Kopiuj

public interface IUserService
{
    Task<bool> Login(LoginData data);
    Task<bool> Logout();
    Task<bool> VerifyEmail(EmailData data);
}

Następnie powinieneś stworzyć sobie na tej podstawie serwis:

Kopiuj

public class UserService: IUserService
{
 //
}

i tutaj mieć całą tą logikę, którą masz w kontrolerze teraz. Razem z obiektem userManager.
Oczywiście musisz pamiętać o tym, żeby ten serwis zarejestrować w dependency injection.

I potem w swoim kontrolerze posługujesz się tylko tym serwisem:

Kopiuj

[HttpPost]
        public async Task<IActionResult> Login(LoginViewModel model)
        {
            //login functionality
           bool loginResult = await userService.Login(model); 
           if(loginResult)
             return RedirectToAction("Index", "Home");
           else
             return RedirectToAction("BadLogin", "Home");
        }

Trochę odświeżam temat :) chcę wyodrębnić "to co się da" do serwisu. No i mam sobie taką metodę w kontrolerze.

[AllowAnonymous]
        public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null, string remoteError = null)
        {
            returnUrl = returnUrl ?? Url.Content("~/");

            LoginViewModel loginViewModel = new LoginViewModel
            {
                ReturnUrl = returnUrl,
                ExternalLogins = (await _signInManager.GetExternalAuthenticationSchemesAsync()).ToList()
            };

            if (remoteError != null)
            {
                ModelState.AddModelError(string.Empty, $"Error from external provider: {remoteError}");
                return View("Login", loginViewModel);
            }

            var info = await _signInManager.GetExternalLoginInfoAsync();
            if (info == null)
            {
                ModelState.AddModelError(string.Empty, "Error loading external login information.");
                return View("Login", loginViewModel);
            }

            var signInResult = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: false, bypassTwoFactor: true);

            if (signInResult.Succeeded)
            {
                return LocalRedirect(returnUrl);
            }
            else
            {
                var email = info.Principal.FindFirstValue(ClaimTypes.Email);

                if (email != null)
                {
                    var user = await _userManager.FindByEmailAsync(email);

                    if (user == null)
                    {
                        user = new ApplicationUser
                        {
                            UserName = info.Principal.FindFirstValue(ClaimTypes.Email),
                            Email = info.Principal.FindFirstValue(ClaimTypes.Email)
                        };

                        await _userManager.CreateAsync(user);
                    }
                    await _userManager.AddLoginAsync(user, info);
                    await _signInManager.SignInAsync(user, isPersistent: false);

                    return LocalRedirect(returnUrl);
                }

                ViewBag.ErrorTitle = $"Nie otrzymano informacji o adresie e-mail od dostawcy: {info.LoginProvider}";
                ViewBag.ErrorMessage = "Proszę skontaktować się z supportem fryzjer@aplikacjafryzjer.com";

                return View("Error");
            }
        }

Z tego co widzę niektóre właściwości są dostępne tylko dla klas dziedziczących po Controller np. ModelState, albo Url. Stąd też tych części nie mogę wyodrębnić. Przynajmniej tak zakładam? Bo chyba bez sensu byłoby aby serwis dziedziczył po klasie Controller :)
No i druga sprawa - w powyższym przykładzie idąc od początku metody, mogę wyodrębnić np. tą część:

            LoginViewModel loginViewModel = new LoginViewModel
            {
                ReturnUrl = returnUrl,
                ExternalLogins = (await _signInManager.GetExternalAuthenticationSchemesAsync()).ToList()
            };

no i załóżmy, że przeniosłem ją do serwisu UserManagerService, muszę teraz zwrócić loginViewModel z serwisu, ponieważ w kontrolerze jest on wykorzystywany do przekazania widoku. Idąc dalej, mogę wyrzucić z kontrolera tą część:

var info = await _signInManager.GetExternalLoginInfoAsync();

Rozumiem, że musiałbym stworzyć w serwisie kolejną metodę, wykonującą tylko tą operację i zwracającą 'info', a następnie tą metodę serwisu wywołać w kontrolerze? Czy to nie trochę sztuka dla sztuki? Czy chodzi o późniejsze utrzymanie kodu?

W takim wypadku zakładam (prośba o poprawę jeśli błędnie), że dla danej metody 'ExternalLoginCallback' będę musiał utworzyć kilka metod w serwisie?

Niekoniecznie potrzebujesz kliku metod w serwisie, kwestia zdefiniowania odpowiedniego typu danych zwracanych z serwisu. Ja bym widział coś takiego:

public class LoginResult
{
    ResultState ResultState { get; }
    string ErrorMessage { get; }
    LoginViewModel Model { get; }
}

Zwracasz to do kontrolera, i kontroler w zależności od enuma ResultState wie, czy ma przekierować, czy dodać error do ModelState czy do ViewBaga.

Poza tym, to wydaje mi się, że ta akcja robi jakieś dziwne rzeczy. Czym jest remoteError i czemu jest argumentem?

(Ten kod, który zaproponowałem da się oczywiście zrobić lepiej jakąś biedamonadą, ale nie chcę przeciążać nadmiarem informacji.)

EDIT:
Przemodelowałem swoje metody wg Twoich uwag, możesz zerknąć czy to jest w porządku?

Metoda w kontrolerze po "odchudzeniu":

               [AllowAnonymous]
        public async Task<IActionResult> ExternalLoginCallback(string returnUrl = null, string remoteError = null)
        {
            Result result = await _userManager.ExternalLoginCallback(returnUrl, remoteError);

            if (result.StateResult == Result.ResultState.Failed)
            {
                ModelState.AddModelError(string.Empty, result.ErrorMessage);
                return View("Login", result.ErrorMessage);
            }
            else if (result.StateResult == Result.ResultState.Interrupted)
            {
                ViewBag.ErrorMessage = result.ErrorMessage;
                return View("Error");
            }
            else
            {
                return LocalRedirect(returnUrl);
            }
        }

Moja metoda w serwisie:

        public async Task<Result> ExternalLoginCallback(string returnUrl, string remoteError)
        {
            Result result = new Result()
            {
                ReturnUrl = returnUrl ?? _urlHelper.Content("~/"),
                Model = new LoginViewModel
                {
                    ReturnUrl = returnUrl,
                    ExternalLogins = (await _signInManager.GetExternalAuthenticationSchemesAsync()).ToList()
                }
            };
            
            if (remoteError != null)
            {
                result.ErrorMessage = $"Error from external provider: {remoteError}";
                result.StateResult = Result.ResultState.Failed;
                return result;
            }

            var info = await _signInManager.GetExternalLoginInfoAsync();

            if (info == null)
            {
                result.ErrorMessage = $"Error loading external login information.";
                result.StateResult = Result.ResultState.Failed;
                return result;
            }

            var signInResult = await _signInManager.ExternalLoginSignInAsync(info.LoginProvider, info.ProviderKey, isPersistent: false, bypassTwoFactor: true);

            if (signInResult.Succeeded)
            {
                result.StateResult = Result.ResultState.Succeeded;
                return result;
            }
            else
            {
                var email = info.Principal.FindFirstValue(ClaimTypes.Email);

                if (email != null)
                {
                    var user = await _userManager.FindByEmailAsync(email);

                    if (user == null)
                    {
                        user = new ApplicationUser
                        {
                            UserName = info.Principal.FindFirstValue(ClaimTypes.Email),
                            Email = info.Principal.FindFirstValue(ClaimTypes.Email)
                        };

                        await _userManager.CreateAsync(user);
                    }
                    await _userManager.AddLoginAsync(user, info);
                    await _signInManager.SignInAsync(user, isPersistent: false);

                    result.StateResult = Result.ResultState.Succeeded;
                    return result;
                }

                result.ErrorMessage = $"Nie otrzymano informacji o adresie e-mail od dostawcy: {info.LoginProvider}";
                result.StateResult = Result.ResultState.Interrupted;
                return result;
            }

Moj typ danych Result:

    public class Result
    {
        public enum ResultState { Invalid, Succeeded, Cancelled, Interrupted, Failed, Pending } 
        public ResultState StateResult { get; set; }
        public string ErrorMessage { get; set; }
        public LoginViewModel Model { get; set; }
        public string ReturnUrl { get; set; }
    }