7 Podstawowych Błędów Programistów Podczas Pisania Testów Jednostkowych

Pisanie dobrych testów jednostkowych nie jest łatwe. W szczególności pierwsze testy mogą sprawiać Ci trochę trudności, także, aby Ci pomóc, chciałem Ci przedstawić 7 moim zdaniem najczęstszych błędów początkujących podczas pisania testów jednostkowych, których powinieneś unikać.

7 Podstawowych Błędów Programistów Podczas Pisania Testów Jednostkowych

1) Złe nazewnictwo metod testowych

Przygotowałem prostą metodę, która sprawdza, czy podany rok jest rokiem przestępnym. Jeżeli tak, to metoda zwraca true, a jeżeli nie, to false.

public class LeapYear
{
    public bool IsLeapYear(int year)
    {
        if (year % 4 != 0)
            return false;

        if (year % 100 == 0 && year % 400 != 0)
            return false;

        return true;
    }
}

Do tej metody napisałem również przykładowy test jednostkowy.

public class LeapYearTests
{
    [Test]
    public void IsLeapYear()
    {
        var leapYear = new LeapYear();

        var result = leapYear.IsLeapYear(2020);

        result.Should().BeTrue();
    }
}

Nazwa metody w klasie testowej jest taka sama jak, w klasie testowanej. Także i tak, jeszcze nie jest ta nazwa najgorsza. Bo spotykałem się z nazwami gorszymi, takie jak Test1 itp. Ale mimo wszystko dalej ta nazwa nie jest dobra, ponieważ nic nam nie mówi o teście. Możemy się tylko domyślać, że testuje metodę IsLeapYear. Ale jeżeli chcemy wiedzieć, co dokładnie testuje, to musimy wgłębić się w cały test. Test nie jest skomplikowany także, akurat w tym przypadku szybko się zorientujemy, co jest testowane, ale co w przypadku bardziej skomplikowanych testów?

Zobaczymy, co się stanie, jeżeli w naszej metodzie testowanej będzie błąd.

public bool IsLeapYear(int year)
{
    if (year % 41 != 0)//celowy błąd
        return false;

    if (year % 100 == 0 && year % 400 != 0)
        return false;

    return true;
}

Po uruchomieniu testu otrzymaliśmy taki komunikat: "IsLeapYear - Expected results to be true, but found False."

Przyznasz, że komunikat jest bardzo ogólny i za dużo nam nie mówi.

Jak w takim razie powinna się nazywać ta metoda? Oczywiście są różne konwencje nazewnictwa testów, ważne, żeby ustalić jedną ze swoim zespołem i taką stosować w całym projekcie. Najgorsze co może być, to jeżeli każdy programista stosuje inną konwencję. Ja zazwyczaj stosuję nazewnictwo, gdzie najpierw podaję nazwę metody, następnie opisuję scenariusz testowany, oraz oczekiwany wynik. Czyli w naszym przypadku, tak może wyglądać nazwa metody testowej:

public class LeapYearTests
{
    [Test]
    public void IsLeapYear_WhenLeapYear_ShouldReturnTrue()
    {
        var leapYear = new LeapYear();

        var result = leapYear.IsLeapYear(2020);

        result.Should().BeTrue();
    }
}

Mając taką nazwę metody, nie musimy już patrzeć do ciała tej metody, ponieważ wiadomo dokładnie, za co ona odpowiada. Jeżeli teraz mamy błąd to od razy widzimy w test explor co i w jakiej metodzie poszło nie tak. Tym razem taki jest komunikat: " IsLeapYear_WhenLeapYear_ShouldReturnTrue - Expected results to be true, but found False."

Sama wiadomość błędu się nie zmienia, ale dzięki nazwie testy – odpowiedniej nazwie testu, od razu wiemy co, jak i gdzie poszło nie tak.

2) Brak assercji w testach jednostkowych

Z moich doświadczeń, że to jest bardzo częsta praktyka. Często w projektach widziałem właśnie takie testy bez żadnych asercji. Przygotowałem metodę, która na podstawie przekazanego argumentu zwraca komunikat, czy liczba jest parzysta, czy nieparzysta:

public class EvenOrOdd
{
    public string GetEvenOrOddMsg(int number)
    {
        if (number % 2 == 0)
            return "Even";

        return "Odd";
    }
}

Mamy już też przygotowany dla metody test jednostkowy (który nie jest dobry).

public class EvenOrOddTests
{
    [Test]
    public void GetEvenOrOddMsg_WhenNumberIsEven_ShouldReturnEven()
    {
        var evenOrOdd = new EvenOrOdd();

        var even = evenOrOdd.GetEvenOrOddMsg(2);
    }
}

Jak widzisz w tym teście, najpierw mamy inicjalizację obiektu, czyli arrange. Następnie mamy act, czyli wywołanie metody testowanej. Jednak na końcu brakuje nam assercji, czyli weryfikacji czy wynik, który zostanie zwrócony w act, będzie prawidłowy. Jeżeli teraz uruchomię ten test, to będzie on zielony, mimo tego, że nie napisaliśmy assercji. Taki brak assercji to bardzo często spotykana praktyka.

Najczęściej wynika ona z 3 rzeczy. Po pierwsze programista mógł zapomnieć o tej assercji, chociaż przyznam, że to mało prawdopodobne, ale mógł np. go ktoś oderwać od pracy, i później zapomniał, że ma nieskończony test. Najgorsze jest to, że testy bez asercji są testami zielonymi, to znaczy przechodzą jako prawidłowe. Po drugie często się zdarza, że programiście nie do końca chce się pisać testy, lub po prosty nie umie, a że biznes wymaga, to wtedy powstaję właśnie takie testy, które tak naprawdę niczego nie testują, ale podwyższają pokrycie kodu. Bo jeżeli sobie podejrzysz % pokrycia kodu testami, to oczywiście taki test podwyższa ten wskaźnik. Po trzecie i wydaje mi się, że to najczęstszy powód takich asercji. Jeżeli programista widzi, że test, który pisał inny programista, nie przechodzi, to zamiast wgłębić się w szczegóły, to po prostu komentuje lub usuwa asercję i wtedy magicznie testy są zielone.

3) Używanie w testach pętli lub instrukcji warunkowych

Kolejny błąd, z którym często można się spotkać, to stosowanie pętli, lub instrukcji warunkowych w testach. Przygotowałem już taki test, znowu dla metody sprawdzającej, czy podana liczba jest parzysta, czy nieparzysta.

[Test]
public void GetEvenOrOddMsg_WhenCalled_ShouldReturnCorrectMsg()
{
    var evenOrOdd = new EvenOrOdd();

    for (int i = 0; i  < 10; i++)
    {
        var msg = evenOrOdd.GetEvenOrOddMsg(i);

        if (i % 2 == 0)
            Assert.That(msg, Is.EqualTo("Even"));
        else
            Assert.That(msg, Is.EqualTo("Odd"));
    }
}

Mamy tutaj pętle, która wykona się 10 razy i w zależności od tego, czy liczba powinna być parzysta, czy nieparzysta, to zostaje zwrócony odpowiedni komunikat. Pomijając to, że tutaj jest powtórzona logika z kodu produkcyjnego, jest to nieczytelne i bezużyteczne, to w przypadku gdy ten test się nie powiedzie, to dostaniemy komunikat, który prawdopodobnie nic nam nie pomoże.

Może najpierw dodam celowy błąd do tego kodu, następnie uruchomimy test i zobaczymy, jaki będzie jego wynik.

[Test]
public void GetEvenOrOddMsg_WhenCalled_ShouldReturnCorrectMsg()
{
    var evenOrOdd = new EvenOrOdd();

    for (int i = 0; i  < 10; i++)
    {
        var msg = evenOrOdd.GetEvenOrOddMsg(i);

        if (i % 2 != 0)//celowy błąd
            Assert.That(msg, Is.EqualTo("Even"));
        else
            Assert.That(msg, Is.EqualTo("Odd"));
    }
}

Po uruchomieniu tego testu oczywiście wynik jest czerwony. Otrzymaliśmy taki komunikat: "GetEvenOrOddMsg_WhenCalled_ShouldReturnCorrectMsg - Exptected string length 3 but was 4. Strings differ at index 0. Expected: Odd But was: Even", który tak naprawdę nic na nie mówi.

Oczywiście ten przykład jest bardzo zły, ale jeżeli chcemy użyć pętli w naszych testach, to prawdopodobnie to samo możemy napisać zgodnie z dobrymi praktykami za pomocą testów parametryzowanych. W NUnit służy do tego atrybut TestCase. Taki test może to wyglądać następująco:

[TestCase(2, "Even")]
[TestCase(4, "Even")]
[TestCase(2, "Odd")]
[TestCase(3, "Odd")]//celowy błąd
public void GetEvenOrOddMsg_WhenCalled_ShouldReturnCorrectMsg(int number, string message)
{
    var evenOrOdd = new EvenOrOdd();

    var result = evenOrOdd.GetEvenOrOddMsg(number);

    result.Should().Be(message);
}

Mamy podobny kod, to znaczy taki, który testuje kilka przypadków, ale jest dużo bardziej czytelny. Nie mamy powtórzonej logiki w teście i jeżeli test się nie powiedzie, to wiemy dokładnie, który przypadek zwrócił nam błąd. Wszystkie te testy są traktowane jako testy oddzielne. W test explorer pojawią się 4 różne błędy i jeżeli pojawi się błąd, to będzie wiedzieć dokładnie, którego przypadku on dotyczy. Przykładowy komunikat o błędzie: "GetEvenOrOddMsg_WhenCalled_ShouldReturnCorrectMsg(2, Odd - Exptected result to bo Odd with a length of 3, but Even has length of 4, differs near Eve (index 0)."

4) Assercje z logiką

Kolejnym błędem jest pisanie asercji, które mają w sobie jakąkolwiek logikę, a co jeszcze gorsza, ta logika jest taka sama jak w metodzie testowanej (co udało mi się zademonstrować również w poprzednim przykładzie). Przygotowałem klasę kalkulator oraz metodę dodawania. Jest to prosta metoda, która zwraca sumę 2 liczb.

public class Calculator
{
    public int Add(int number1, int number2)
    {
        return number1 + number2;
    }
}

Mamy również test jednostkowy dla tej metody (który jest zły):

public class CalculatorTests
{
    [Test]
    public void Add_WhenCalled_ShouldReturnSum()
    {
        var calculator = new Calculator();

        var result = calculator.Add(1, 2);

        result.Should().Be(1 + 2);
    }
}

W assercji użyliśmy obliczeń. Dodatkowo tych samych, co w metodzie testowanej. Przez co tak naprawdę ten test staje się bezużyteczny. Do asercji powinny być przekazywane już konkretne wyniki, a nie obliczane dopiero w assercji. Czyli w naszym przykładzie, powinno tam być na sztywno wpisane 3 i wtedy assercja jest prawidłowa. Czyli test powinien wyglądać tak:

public class CalculatorTests
{
    [Test]
    public void Add_WhenCalled_ShouldReturnSum()
    {
        var calculator = new Calculator();

        var result = calculator.Add(1, 2);

        result.Should().Be(3);
    }
}

5) Testowanie tylko optymistycznej ścieżki

Przygotowałem metodę, która dzieli nam przez siebie 2 przekazane do metody liczby.

public class Calculator
{
    public int Divide(int dividend, int divisor)
    {
        if (divisor == 0)
            throw new DivideByZeroException();

        return dividend / divisor;
    }
}

Wewnątrz metody mamy również sprawdzenie dzielnika. W przypadku, jeżeli dzielnik jest 0, to zostanie rzucony odpowiedni wyjątek. Na koniec wykonywane są obliczenia.

Tak może wyglądać przykładowy test:

public class CalculatorTests
{
    public void Divide_WhenCalled_ShouldReturnDivision()
    {
        var calculator = new Calculator();

        var result = calculator.Divide(4, 2);

        result.Should().Be(2);
    }
}

Ta metoda jest dobrze napisana, ale mamy tutaj 1 błąd. Testowana jest tylko optymistyczna ścieżka, ale to nie jest wystarczające. Powinno się również przetestować różne warunki brzegowe. W szczególności powinien nas interesować warunek, gdy dzielnik będzie 0. W takim przypadku powinien zostać rzucony wyjątek DivideByZeroException.

public class CalculatorTests
{
    public void Divide_WhenCalled_ShouldReturnDivision()
    {
        var calculator = new Calculator();

        var result = calculator.Divide(4, 2);

        result.Should().Be(2);
    }

    public void Divide_WhenDivisorIsZero_ShouldThrowDivideByZeroException()
    {
        var calculator = new Calculator();

        Action action = () => calculator.Divide(4, 0);

        action.Should().ThrowExactly <DivideByZeroException>();
    }
}

Także, trzeba pamiętać nie tylko o tym, by testować optymistyczne ścieżki, ale również wszystkie warunki brzegowe, dopiero wtedy można powiedzieć, że metoda jest odpowiednio przetestowana. Dodatkowo musisz pamiętać, że czasem testujemy nie tylko to, co się ma wydarzyć, ale również to, co ma się nie wydarzyć. To znaczy, jeżeli testujesz metodę, która pobiera zamówienia tylko od konkretnego użytkownika, to należy przetestować czy przypadkiem nie zwraca również zamówień od innych użytkowników.

6) Próba testowania wszystkiego

Kolejny błąd to próba testowania wszystkiego, nawet trywialnego kodu, takiego jak właściwości, które tylko przypisują wartości zmiennych lub konstruktory, czy na przykład jakiś kod generowany automatycznie. Testowanie takich przypadków jest zbędne, nie przynosi nam żadnych korzyści. Jedyne co to nam zwiększa procent pokrycia kodu w aplikacjach, ale jak już pokazałem wcześniej, te mierniki pokrycia kodu nie są zbyt dobre. Można bez problemu bezużytecznymi testami zwiększyć to pokrycia, ale nie przyniesie nam to w przyszłości żadnej korzyści. Dlatego proponuje Ci zrezygnować z takich pomiarów, bo prawda jest taka, że nic one nam nie mówią. Kiedyś słyszałem ciekawe zdanie, które mówiło o tym, że są tylko dwa pomiary pokrycia kodu, które nam coś mówią. Jest to pokrycie 0% oraz 100%. Pokrycie kodu 0% mówi nam o tym, że testów jest za mało, a pokrycie kodu w 100% mówi nam o tym, że testów jest za dużo.

7) Nadużywanie mocków

Bardzo częsty błąd, który chciałem Ci jeszcze przedstawić, dotyczy sytuacji, gdy programiści mockują prawie wszystko w swoich testach. Nie powinniśmy mockować wtedy, gdy nie trzeba, mocki służą tylko do pozbywania się zewnętrznych zależności w naszych testach. Przejdźmy do przykładu.

public class OrderService
{
    public decimal CalculateDiscount(decimal orderAmount, Customer customer)
    {
        if (customer == null)
            throw new ArgumentNullException(nameof(customer));

        if (customer.IsNewCustomer)
            return 0;

        if (orderAmount > 100)
            return 20;

        return 0;
    }
}

public class Customer
{
    public int Id { get; set; }
    public bool IsNewCustomer { get; set; }
}

Przygotowałem klasę OrderService. Mamy tutaj jedną publiczną metodę CalculateDiscount, która na podstawie przekazanego klienta, oraz kwoty zamówienia, zwraca wartość zniżki. Dobrze napisany Test jednostkowych do takiej metody może wyglądać w ten sposób:

public class OrderServiceTests
{
    [Test]
    public void CalculateDiscount_WhenCustomerIsNew_ShouldReturn0()
    {
        var orderService = new OrderService();

        var discount = orderService.CalculateDiscount(1m, new Customer { IsNewCustomer = true });

        discount.Should().Be(0);
    }
}

Oczywiście to jest tylko test do jednego przypadku, który chcemy przetestować. Nie będziemy analizować wszystkich przypadków. Także przyjrzymy się tylko temu jednemu przypadkowi, to znaczy czy jeśli klient jest nowy klientem, to zniżka będzie wynosić zero. Ten test jest jak najbardziej dobry. Po uruchomieniu testu wynik jest zielony, czyli test przechodzi poprawnie.

Teraz pokaże Cim w jaki sposób nie powinno się pisać testów do takiego przypadku. Często spotykam się z testami, gdzie prawie wszędzie są podstawiane mocki. Zobacz, jak wtedy takie testy wyglądają.

Po pierwsze zamiast klasy Customer, jest tworzony nowy interfejs ICustomer.

public interface ICustomer
{
    int Id { get; set; }
    bool IsNewCustomer { get; set; }
}

Klasa Customer implementuje ten interfejs:

public class Customer : ICustomer
{
    public int Id { get; set; }
    public bool IsNewCustomer { get; set; }
}


public class Customer : ICustomer
{
    public int Id { get; set; }
    public bool IsNewCustomer { get; set; }
}

Do metody CalculateDiscount jako parametr jest przekazywany właśnie stworzony interfejs:

public class OrderService
{
    public decimal CalculateDiscount(decimal orderAmount, ICustomer customer)
    {
        if (customer == null)
            throw new ArgumentNullException(nameof(customer));

        if (customer.IsNewCustomer)
            return 0;

        if (orderAmount > 100)
            return 20;

        return 0;
    }
}

Dzięki czemu można tę właściwość zamockować. I test w takim przypadku może wyglądać w ten sposób:

public class OrderServiceTests
{
    [Test]
    public void CalculateDiscount_WhenCustomerIsNew_ShouldReturn0()
    {
        var orderService = new OrderService();

        var discount = orderService.CalculateDiscount(1m, new Customer { IsNewCustomer = true });

        discount.Should().Be(0);
    }

    [Test]
    public void CalculateDiscount_WhenCustomerIsNew_ShouldReturn0_Mock()//zły przykład
    {
        var customer = new Mock <ICustomer>();
        customer.Setup(x => x.IsNewCustomer).Returns(true);
        var orderService = new OrderService();

        var discount = orderService.CalculateDiscount(1m, customer.Object);

        discount.Should().Be(0);
    }
}

Co prawda po uruchomieniu oba testy przechodzą. Niby oba testy wydają się, że są w porządku. Jednak porównaj te 2 testy ze sobą, po pierwsze test z użyciem mocka, jest dużo mniej czytelny. Jeżeli przykład byłby jeszcze bardziej skomplikowany, to inicjalizacja mocka jeszcze bardziej zmniejszyłaby czytelność tego testu. Po drugie musieliśmy stworzyć nowy interfejs dla klasy Customer, tak aby była możliwość jej zamockowania, a w tej sytuacji jest to zupełnie niepotrzebne.

Także, podsumowując, najlepiej mockować tylko zewnętrzne zależności, to znaczy kontakt z bazą danych, plikami, webserwisami itd. Jeżeli mockujemy zbyt wiele, to po pierwsze nasze klasy testowe staja się obszerniejsze, bardziej skomplikowane, a powinny być krótkie, czytelne i proste. Jeżeli chcemy mockowac za dużo, to musimy tworzyć wiele interfejsów. Każda taka klasa musi mieć swój własny interfejs, często tylko dlatego, żeby napisać do niej testy. Jeżeli przekazujemy dużo takich zależności, zazwyczaj poprzez konstruktory, to wtedy te konstruktory mają dużo parametrów, a to tez nie jest dobre. Musisz pamiętać, że każdy mock wprowadza do testów zakłamanie, a najlepiej takich zakłamań mieć w testach jak najmniej. Także, najczęściej powinno się mockowac tylko zewnętrzne zależności, są oczywiście wyjątki od tej reguły. Jeżeli np. mamy 2 klasy, jedna z nich używa drugiej i przez to mamy mnóstwo nowych przypadków testowych, to czasem jednak warto zastosować mocka dla takich przypadków. U nas natomiast, w przykładzie powyżej mocki są zbędne.

PODSUMOWANIE

Podsumowując, 7 moim zdaniem najczęstszych błędów początkujących podczas pisania testów jednostkowych to:

Złe nazewnictwo metod testowych.
Brak assercji.
Używanie w testach pętli lub instrukcji warunkowych.
Asercje z logiką.
Testowanie tylko optymistycznej ścieżki.
Próba testowania wszystkiego.
Nadużywanie mocków.

Staraj się proszę unikać przynajmniej tych 7 podstawowych błędów. Dzięki temu twoje testy jednostkowe będą dobre i przyniosą dużo wartości w Twoim projektach.

To wszystko na dzisiaj, do zobaczenia w kolejnym artykule.

Poprzedni artykuł - 19 Najczęściej Używanych Metod Stringa, Które Warto Znać .
Następny artykuł - FluentAssertions – Płynne Assercje w Testach Jednostkowych.