Największy wróg unit testów - logika.

Logika, przepływ sterowania, instrukcje warunkowe - to dzięki nim nasze programy mogą spełniać szeregi wymagań i oczekiwań. Dzięki nim możemy rozbudowywać nasz program, rozwijać obecne funkcjonalności oraz dodawać nowe. Każdy programista wie, że bez nich tworzenie oprogramowania nie mogłoby mieć miejsca. Czy zatem nasze unit testy, również powinny zawierać logikę? Zdecydowanie nie, i w tym wpisie wyjaśnię dlaczego, tym razem odpowiedź jest taka klarowna, a nie, jak to nieraz w naszej branży bywa - “to zależy”.

Logika zwiększa ryzyko błędu

Każde rozgałęzienie procesu przetwarzania w naszym kodzie zwiększa jego podatność na błędy. Unit testy to narzędzie do weryfikowania założeń względem pisanego przez nas oprogramowania. Powinny one być jasno zdefiniowane. Jeśli w testach pojawia się logika to najczęściej jest to droga na skróty, by zaoszczędzić trochę czasu na dokładniejsze przeanalizowanie wymagań względem konkretnych scenariuszy użycia testowanego kodu.

Wyobraźmy sobie klasę do naliczania rabatów DiscountCalculator. Zamiast napisać trzy proste testy, w poniższym “sprytnym” przykładzie użyta została nie tylko pętla for, ale również instrukcje warunkowe if/else, aby przetestować różne progi rabatowe w jednym miejscu.

TEST_F(DiscountCalculatorTest, calculate_CheckAllDiscounts_ShouldApplyCorrectRates)
{
    const std::vector<double> purchases{ 50.0, 150.0, 300.0 };
    DiscountCalculator calculator{};

    for (const auto& amount : purchases)
    {
        const auto result{ calculator.calculate(amount) };

        if (amount < 100.0)
        {
            EXPECT_NEAR(0.0, result, 0.001);
        }
        else if (100.0 <= amount && amount < 200.0)
        {
            EXPECT_NEAR(amount * 0.1, result, 0.001);
        }
        else
        {
            EXPECT_NEAR(amount * 0.2, result, 0.001); 
        }
    }
}

Dodanie logiki do testu powoduje szereg problemów. Jeśli test nie przejdzie, to nie tylko musimy debugować nasz kod, ale również kod testu. A tutaj prawidłowo napisane testy dla metody calculate.

TEST_F(DiscountCalculatorTest, calculate_TooLowAmount_ShouldReturnNoDiscount)
{
    const auto amount{ 50.0 };
    DiscountCalculator calculator{};

    const auto result{ calculator.calculate(amount) };

    EXPECT_NEAR(0.0, result, 0.001);
}

TEST_F(DiscountCalculatorTest, calculate_FirstThresholdAmount_ShouldReturn10PercentDiscount)
{
    const auto amount{ 150.0 };
    DiscountCalculator calculator{};

    const auto result{ calculator.calculate(amount) };

    EXPECT_NEAR(15.0, result, 0.001);
}

TEST_F(DiscountCalculatorTest, calculate_SecondThresholdAmount_ShouldReturn20PercentDiscount)
{
    const auto amount{ 300.0 };
    DiscountCalculator calculator{};

    const auto result{ calculator.calculate(amount) };

    EXPECT_NEAR(60.0, result, 0.001);
}

Jeśli unit testy nie mają prostej sekwencyjnej struktury tylko rozgałęziają się na różne scenariusze, to tracimy jedną z najważniejszych cech dobrych unit testów, powtarzalność. Tak w zasadzie to logika w testach łamie również inną zasadę z akronimu F.I.R.S.T. - I. Testy, przez wprowadzone instrukcje warunkowe, mogą utracić swoją niezależność względem innych testów. Zwłaszcza, gdy korzystają ze wspólnej metody narzędziowej klasy test siuty.

Dla przykładu testujemy klasę OrderProcessor. Mamy metodę narzędziową setupMocks, która w zależności od przekazanych flag, konfiguruje mocki dla różnych scenariuszy.

class OrderProcessorTest : public testing::Test
{
protected:
    // Wspólna metoda narzędziowa z ukrytą logiką sterującą
    auto setupProcessor(bool isGoldCustomer, bool hasDiscount) -> void
    {
        if (isGoldCustomer)
        {
            EXPECT_CALL(customerServiceMock_, getStatus(_)).WillRepeatedly(Return(Status::Gold));
        }

        if (hasDiscount)
        {
            EXPECT_CALL(discountServiceMock_, apply(_)).WillOnce(Return(true));
        }
        else
        {
            // Brak zniżki może zmieniać stan mocka w sposób, 
            // którego inny test się nie spodziewa
            EXPECT_CALL(discountServiceMock_, apply(_)).Times(0);
        }
    }

    MockCustomerService customerServiceMock_;
    MockDiscountService discountServiceMock_;
};

TEST_F(OrderProcessorTest, process_GoldCustomerWithoutDiscount_ShouldWork)
{
    setupProcessor(true, false);
    OrderProcessor processor{ customerServiceMock_, discountServiceMock_ };

    const auto result{ processor.process(Order{}) };

    EXPECT_TRUE(result.success);
}

To jest dokładnie ta sytuacja, o której wspominałem we wpisie o DRY i innych zasadach programowania. Nie zawsze stosowanie DRY w testach to dobry pomysł. Poniżej poprawna wersja.

class OrderProcessorTest : public testing::Test
{
protected:
    auto setupProcessorForCustomerWithoutDiscount(const Status customerStatus) -> void
    {
        EXPECT_CALL(customerServiceMock_, getStatus(_)).WillRepeatedly(Return(customerStatus));
    }

    auto setupProcessorForCustomerWithDiscount(const Status customerStatus) -> void
    {
        EXPECT_CALL(customerServiceMock_, getStatus(_)).WillRepeatedly(Return(customerStatus));
        EXPECT_CALL(discountServiceMock_, apply(_)).WillOnce(Return(true));
    }
  
    MockCustomerService customerServiceMock_;
    MockDiscountService discountServiceMock_;
};

TEST_F(OrderProcessorTest, process_GoldCustomerWithoutDiscount_ShouldWork)
{
    setupProcessorForCustomerWithoutDiscount(Status::Gold);
    OrderProcessor processor{ customerServiceMock_, discountServiceMock_ };

    const auto result{ processor.process(Order{}) };

    EXPECT_TRUE(result.success);
}

W tym przykładzie moglibyśmy w zasadzie, po prostu usunąć metody narzędziowe i bezpośrednio dodawać EXPECT_CALL do testów. Są jednak przypadki, gdzie takich instrukcji jest więcej i wtedy warto rozważyć stworzenie metody narzędziowej. To bardzo dobra technika, ciało testu się skraca, a dodatkowo dobrze dobrana nazwa metody dodaje więcej kontekstu. Pamiętaj tylko bez logiki! ;)

Powielanie błędów z kodu produkcyjnego

Częstym powodem dodawania logiki do unit testów jest chęć odwzorowania przepływu sterowania w testowanym kodzie. Naraża nas to jednak na powielenie błędów z produkcyjnego kodu. Unit testy powinny wykrywać defekty, nie powielać logikę doprowadzając do sytuacji, w której testy w zasadzie są kopią testowanego kodu.

Tym razem mamy klasę LoyaltyPointsCalculator, liczymy punkty lojalnościowe. Kto nie lubi ekstra rabatów ;) Logiką, w tym przypadku, jest kopia algorytmu z kodu produkcyjnego. Nie ma tam wprawdzie ifa, niemniej zamiast podać konkretną liczbę, nasz test liczy sobie oczekiwaną wartość podczas swojego wykonywania.

TEST_F(LoyaltyPointsTest, calculatePoints_LargeOrder_ShouldReturnPointsWithBonus)
{
    const double orderValue{ 600.0 };
    const double baseRate{ 10.0 };
    const double bonusMultiplier{ 1.05 };
    LoyaltyPointsCalculator calculator{};

    EXPECT_NEAR((orderValue / baseRate) * bonusMultiplier, calculator.calculate(orderValue), 0.001);
}

To nic innego jak fragment implementacji przeklejony do testu. Nie mamy żadnej gwarancji, że nasz test nie odziedziczył przypadkiem logicznego błędu, który miał przecież wykrywać. W asercji powinna się znaleźć pożądana wartość, a nie sposób jej wyliczania. Poniżej poprawiony test.

TEST_F(LoyaltyPointsTest, calculatePoints_LargeOrder_ShouldReturnPointsWithBonus)
{
    const auto orderValue{ 600.0 };
    LoyaltyPointsCalculator calculator{};

    const auto result{ calculator.calculate(orderValue) };

    EXPECT_NEAR(63.0, result, 0.1);
}

Możliwe, że zastanawiasz się jeszcze dlaczego podawać już obliczoną wartość. Przecież i tak trzeba było ją policzyć. Tak, zgadza się, jednak takich testów z pewnością napisalibyśmy więcej. Jeśli za każdym razem przekopiowalibyśmy logikę z kodu produkcyjnego do testów, to nic by one nie wykryły. Błąd natomiast zapewne wykryliby użytkownicy, co było by znacznie bardziej kosztowne i bolesne. Jeśli obliczamy wartości sami i wpisujemy je bezpośrednio do naszych testów to istnieje dużo mniejsza szansa, że za każdym razem obliczymy ją niepoprawnie.

Zmniejszenie czytelności

Logika, na przykład w postaci bloku switch/case może utrudniać nam odczytanie intencji testów. Nazwy są niejasne, mało konkretne. Odzwierciedlają tylko to, że pojedynczy unit test testuje więcej niż jeden scenariusz. Dlatego tak ciężko dobrać prawidłową nazwę. Tracimy kontekst bo jest on rozmywany przez logikę, która steruje częścią asercji w zależności od tego co testowana metoda zwróci.

Dla przykładu mamy klasę OrderNotifier, która między innymi nadaje priorytety otrzymanym zamówieniom. W zależności od statusu, priorytet będzie inny. Poniżej przykład wszechstronnego, “mega” unit testu.

TEST(OrderNotifierTest, determinePriority_MultipleStatuses_ShouldReturnCorrectPriority)
{
    std::vector<OrderStatus> statuses{ OrderStatus::New, OrderStatus::Shipped, OrderStatus::Cancelled };
    OrderNotifier notifier{};

    for (const auto& status : statuses)
    {
        const auto priority{ notifier.determinePriority(status) };

        switch (status)
        {
            case OrderStatus::New:
                EXPECT_EQ(Priority::Low, priority);
                break;
            case OrderStatus::Shipped:
                EXPECT_EQ(Priority::High, priority);
                break;
            case OrderStatus::Cancelled:
                EXPECT_EQ(Priority::Critical, priority);
                break;
        }
    }
}

Pomimo, iż sama nazwa trzyma się standardu triple A to nie za wiele to daje. Nazwa jest po prostu zbyt ogólna. Jedyne czego możemy być pewni, to, że testuje metodę determinePriority klasy OrderNotifier. Z pewnością przyznasz, że to mało konkretne ;) Ciało testu również nie trzyma standardu AAA. Bloki Act i Assert są wymieszane. Jeśli test zfailuje to w logach będziemy musieli szukać odpowiedzi. Gdyby to były osobne testy, po samej nazwie wiedzielibyśmy, co poszło nie tak.

Zobacz jak to powinno wyglądać poprawnie.

TEST_F(OrderNotifierTest, determinePriority_NewOrder_ShouldReturnLowPriority)
{
    OrderNotifier notifier{};

    const auto priority{ notifier.determinePriority(OrderStatus::New) };

    EXPECT_EQ(Priority::Low, priority);
}

TEST_F(OrderNotifierTest, determinePriority_OrderShipped_ShouldReturnHighPriority)
{
    OrderNotifier notifier{};

    const auto priority{ notifier.determinePriority(OrderStatus::Shipped) };

    EXPECT_EQ(Priority::High, priority);
}

TEST_F(OrderNotifierTest, determinePriority_OrderCancelled_ShouldReturnCriticalPriority)
{
    OrderNotifier notifier{};

    const auto priority{ notifier.determinePriority(OrderStatus::Cancelled) };

    EXPECT_EQ(Priority::Critical, priority);
}

I teraz to się nazywają czytelne testy! Prawda? Wszystko widoczne jak na tacy. Proste i skuteczne.

Dopuszczalne rodzaje logiki

Istnieją pewne elementy logiki, które nie mają negatywnych skutków dla naszych unit testów i wręcz mogą poprawić ich czytelność - pętle. Ich stosowanie z pewnością będzie wskazane przy weryfikacji kontenerów. Niemniej, za każdym razem, gdy chcemy użyć pętli w unit testach powinniśmy sobie zadać pytanie: czy dodanie pętli jest niezbędne, by zachować czytelność i prostotę weryfikacji lub do realizacji konkretnego scenariusza testowego? Jeśli odpowiedź brzmi tak, śmiało użyj pętli.

TEST_F(ReportBatchProcessorTest, processReports_AllValidReports_EachReportIsMarkedAsProcessedAndNoErrors)
{
    const std::vector<Report> reports{ { "ID_1" },{ "ID_2" },{ "ID_3" } };
    ReportBatchProcessor processor{ reports };

    processor.processReports();

    const auto results{ processor.getProcessedReports() };
    ASSERT_THAT(results, testing::SizeIs(reports.size()));
    for (const auto& report : results)
    {
        EXPECT_THAT(report.isProcessed(), testing::IsTrue());
        EXPECT_THAT(report.errorCode(), testing::Eq(0));
    }
}

W powyższym przykładnie zastosowanie pętli for jak najbardziej jest poprawne. Moglibyśmy wprawdzie po prostu napisać 6 asercji zamiast dwóch w pętli. Jednak jeśli elementów w kontenerze byłoby więcej, kod testu niepotrzebnie by się wydłużył. Zauważ przy okazji jakich asercji użyłem: ASSERT_THAT i EXPECT_THAT. Znasz je może? Ja wcześniej nie znałem. Po kilku testach stwierdziłem, że warto się im przyjrzeć bliżej. Z pewnością przygotuje osobny post o optymalizacji czytelności wyników testów, gdy się niepowiodą, tak, aby sam komunikat Google Testa jasno wskazywał nam co poszło nie tak.

Podsumowanie

Mam nadzieję, że tym wpisem przekonałem Cię do niedodawania logiki to Twoich unit testów oraz uważniejszego code review swoich kolegów i koleżanek z zespołu. Ja sam wciąż zbyt często spotykam się z tym problemem i staram się dzielić wiedzą o tym jak pisać dobrej jakości unit testy. Co myślisz o tak rygorystycznym podejściu do braku logiki w unit testach? Uważasz to za przesadę, a może się ze mną zgadzasz? Koniecznie podziel się swoją opinią w komentarzu :)

Autor: Tadeusz Biela
Programista C++ | Entuzjasta TDD | Fan unit testów

LinkedIn