Testy jednostkowe: strict vs loose mocks

2

Jakiś czas temu w pracy mieliśmy dużą dyskusję na temat sposobu tworzenia mocków. Gorącym tematem okazał się poziom restrykcyjności mocków. Ustaliliśmy w końcu, że będziemy używać loose, jednak ja nie do końca poczułem się przekonany (wcześniej przez kilka lat używałem strict, może to dlatego). W skrócie: mock strict rzuci wyjątek przy każdym wywołaniu nieskonfigurowanej lub niepoprawnie skonfigurowanej metody, mock loose w takim przypadku zwróci null. W praktyce przekłada się to na to, że

  • podejście strict - jak sama nazwa wskazuje - jest dużo bardziej restrykcyjne, bo musi być dokładnie zasetupowana każda zależność (z dokładnością do kruczków typu It.IsAny<T>). To oznacza, że pisząc jeden test przy okazji sprawdzamy więcej rzeczy, niż by wynikało z samego testu. To często pozwala wykryć własne błędy, acz zbyt dalekie pójście w tym kierunku, czyli pisanie minimum testów, może oznaczać przegapienie pewnych istotnych ścieżek wykonania. Jest to też trochę upierdliwe przy refactoringu albo modyfikacji istniejących funkcjonalności, bo często trzeba poprawiać dziesiątki testów sprawdzających kompletnie co innego niż to,co zmieniliśmy w testowanej metodzie. Ale skoro zmieniliśmy coś w działaniu metody, no to na chłopski rozum jej testy powinny się wywalić. To się nazywa testowanie implementacji.
  • podejście loose umożliwia setup tylko testowanej ścieżki wykonania - skonfigurowane są tylko te metody, które są niezbędne do poprawnego wykonania testu. Jeśli dodamy nową funkcjonalność do testowanej metody (np. weryfikacja uprawnień użytkownika), to istnieje duża szansa, że test się nie wywali, o ile tylko nie rozwalimy ścieżki prowadzącej do sprawdzenia testowanej funkcjonalności. To podejście też jest fajne, bo przy modyfikowaniu istniejących rzeczy wywali się nam mało testów albo wręcz żaden, ale wywołuje to we mnie pewną nieufność ;-) Przy refactoringu jest więcej roboty, bo powstaje więcej testów (strict może testować wiele ścieżek, loose tylko jedną, więc musi być więcej testów w tym drugim przypadku). Więcej testów niekoniecznie jest wadą, bo są one bardziej szczegółowe, ale też łatwiej coś przegapić. To podejście nazywa się testami funkcjonalności.

Po tym długim wstępie pytanie - którą drogą idziecie w waszych projektach i dlaczego?

2

Loose. Przygotowanie testu jednostkowego powinno obejmować to czego test ma dotyczyć. Wykrywanie rzeczy po drodze IMHO jest świetne, ale w bardziej ogólnych testach np integracyjnych. Jeżeli chce przetestować kilka rzeczy to piszę kilka testów.

3
ŁF napisał(a):

Ale skoro zmieniliśmy coś w działaniu metody, no to na chłopski rozum jej testy powinny się wywalić. To się nazywa testowanie implementacji.

Moim zdaniem to jest słabe i staram się tego unikać. W testach chcę sprawdzić, czy jednostka robi to, czego od niej oczekuję, a sposób na osiągnięcie celu jest dla mnie zazwyczaj nieistotny. Jeżeli mam metodę dodającą wsad do kolekcji, to interesuje mnie, czy po wykonaniu metody dane są w kolekcji, a nie to, czy były one dodawane pojedynczo czy hurtem. Ogólnie to bardzo nie lubię mock.verify, zawsze mam wrażenie, że jeżeli tak muszę testować swój kod, to skopałem implementację.

Ergo: podejście loose, bo interesuje mnie efekt metody, a nie weryfikowanie każdego jej kroku w teście.

0

zalezy

czasami strict czasami loose

Loose uzywam gdy nie chce sprawdzic z jakimi parametrami metoda zostala wywolana bo mnie nie interesuje co zostalo wywolane tylko np wynik funkcji
Stritc uzywam, gdy chce sprawdzic z jakimi parametrami dany mock zostal wywolany (mock.Verify())

w kodzie w ktorym pracuje, malo dosc malo jest sprawdzania nulli w polaczeniu z interfejsami. Dlatego loose wywali test tak samo jak strict

4

Loose poza jakimiś wyjątkowymi sytuacjami kiedy chcesz robic sobie quasi-formalną weryfikacje algorytmu.
Robienie takiego uber setupu z ustalaniem kolejności metod itd to generalnie cementowanie kodu, bo potem każda zmiana w kodzie wymaga poprawiania testu, mimo że wcale nic nie psuje.
Są dwa rodzaje złego testu i oba należy wywalić z kodu:

  1. Test który nic nie sprawdza -> popsucie kodu nie powoduje wywalenia się testu. Taki test jest bezużyteczny a do tego daje fałszywe poczucie bezpieczeństwa u developera, bo testy zielone
  2. Test który sprawdza czy aktualny kod wygląda tak samo jak kiedy pisano test -> kosmetyczna modyfikacja kodu wywala test, mimo że wszystko działa. Taki test znów jest bezużyteczny bo raz że sprawdza czy kod jest identyczny jak ten opisany w teście, a nie czy zachowuje sie tak samo, a dwa że rodzi frustracje kiedy coś chcemy zmienić.
8

Ja jestem w ogólności przeciwnikiem mockowania, bo typowe użycie bibliotek typu Mockito skutkuje tym, że mamy bardzo duże pokrycie kodów testami, ale testy szybko stają się bezużyteczne. Pisałem już tu chyba o tym kiedyś. Sprawa ogólnie polega na tym, że mockowanie polega na zasymulowaniu mockowanej klasy według jej aktualnego kontraktu. Jeśli kontrakt klasy się zmieni to by wszelkie mocki tej klasy były poprawne trzeba je zaktualizować. Nic tego jednak nie wymusza, bo testy są zielone.

Problem opiszę na przykładzie. Załóżmy, że mamy klasę:

class Incrementer {
  int increment(int x) {
    return x + 1;
  }
}

Test jednostkowy do tej klasy jest trywialny i nie muszę go pokazywać. Klasa Incrementer ma wielu klientów (czyli klas korzystających z Incrementera). W testach jednostkowych dla klientów Incrementera sam Incrementer jest mockowany.

Przykładowa klasa będąca klientem Incrementera:

class HitCounter {
  Incrementer incrementer;
  int hitCount;

  void recordHit() {
    hitCount = incrementer.increment(hitCount);
  }
  
  int getHitCount() {
    return hitCount;
  }
}

W teście dla HitCountera mockujemy Incrementera:

void testHitCountRecording() {
  Incrementer incrementer = mock(Incrementer.class);
  HitCounter hitCounter = new HitCounter();
  hitCounter.incrementer = incrementer;
  when(incrementer.increment(0)).thenReturn(1);
  hitCounter.recordHit();
  assert(hitCounter.getHitCount() == 1);
}

Jak na razie wszystko spoko. Po jakimś czasie zmieniamy kontrakt Incrementera i nowy wygląd metody increment wygąda tak:

  int increment(int x) {
    return x + 2;
  }

Sypie się tylko test jednostkowy dla Incrementera, więc go poprawiamy. Testy jednostkowe dla wszystkich pozostałych klas jednak się nie psują, bo zamockowaliśmy wywołania Incrementera w tychże testach. Sytuacja końcowa jest więc taka, że mimo iż wszystkie testy są zielone, a pokrycie kodu testami jest wysokie to nasze testy praktycznie nic nie wykrywają.

Powyższy przykład oczywiście jest mocno z czapy, ale tu chodzi o przekazanie idei, a nie zaciemnianie jej szczegółami. Z drugiej strony można jednak podać inne trywialne przykłady dla lepszego zobrazowania.

Załóżmy, że mamy klasę Cache będącą mapą i w niej metodę dodającą obiekt do mapy:

class Cache {
  Map<String, Object> cache;

  void add(String name, Object value) {
    cache.put(name, value);
  }
}

Klasa Cache jest używana w wielu miejscach i w testach jednostkowych wywołania metody add są mockowane. Po pewnym czasie zmieniamy jednak kontrakt metody Cache.add. Wygląda ona teraz następująco:

  void add(String name, Object value) {
    if (cache.contains(name)) {
      throw new IllegalArgumentException();
    } else {
      cache.put(name, value);
    }
  }

Teraz wystarczy poprawić tylko test jednostkowy dla klasy Cache i wszystkie testy świecą się na zielono. Po odpaleniu programu jednak sypie się od razu. Dlaczego? Dlatego, że wszystkie zamockowane użycia metody Cache.add zostały skonstruowane według starego kontraktu, w którym metoda Cache.add nie rzucała wyjątku.

Mockowanie dla mnie jest w zasadzie złem koniecznym, gdy niepraktyczne jest podejście idealistyczne. A podejście idealistyczne polega na tym, że każda klasa która jest zasobożerna (tzn dla której postawienie testowej instancji jest niemożliwe lub znacznie by te testy wydłużyło) powinna mieć lekką testową implementację, której kontrakt jest identyczny z klasą produkcyjną (tzn nie-testową). Klasy produkcyjne, które już są lekkie nie potrzebują lekkich zamienników i mogą być wykorzystywane wprost w testach.

Robienie testowych lekkich implementacji jest na pewno możliwe dla logiki wewnątrz projektu, a więc np warstwy dostępu do danych (logikę dobijającą się do prawdziwej bazy danych można zastąpić logiką operującą na kolekcjach w pamięci RAM). Testowe implementacje dla zewnętrznych usług (np osobny mikroserwis czy serwer) to już w zasadzie droga donikąd, więc w takim przypadku (API używanego przez zewnętrze serwisy) trzeba trzymać się zasady, że raz wystawiona końcówka (np RESTowa) nigdy nie zmienia swojego kontraktu. Brak zmian w kontrakcie (semantyce) oznacza, że mockowanie takich zewnętrznych endpointów nie będzie powodować problemów opisanych na początku postu.

Mam nadzieję, że zrozumiecie o co mi chodzi :)

1

Ja jestem leniwy, więc u mnie mocki tworzą się same, od razu mają zamockowane wszystkie metody zwracające sensowne dane (nie w sensie biznesowym, ale po prostu są wypełnione). Zalety są takie, że:

  • sam piszę niewiele kodu, ale bez problemu mogę sprawdzić, która metoda została wywołana ile razy i z jakimi parametrami (tylko, że rzadko ma to sens);
  • jeśli dodam gdzieś nową zależność, to nie wysypią mi się fałszywie testy, które co prawda nie korzystają z tej zależności w swojej ścieżce, ale muszą ją mieć zamockowaną;
  • sensowne dane mockuję tylko dla przebiegu, który faktycznie chcę przetestować, cała reszta mnie nie obchodzi;
  • testy są krótkie i czytelne, cała faza "arrange" jest tak krótka, że praktycznie nie istnieje, w ekstremalnym przypadku może to być tylko utworzenie SUT.

Ale ogólnie z mockami nie mam problemu - bo używam ich tylko do mockowania zewnętrznych zależności oraz rzeczy specyficznych takich jak np. cache - tu trzeba sprawdzić, czy zależność jest wywoływana dokładnie raz.
Generalnie jak trafiam do jakiegoś projektu, to masę mocków usuwam, bo:

  • zamockowane zachowanie to dokładne powielenie metody rzeczywistej, więc istnienie takiego mocka nie ma sensu;
  • test sprawdza czy framework mockujący działa - nie ma żadnej logiki ani przepływu, po prostu asercja na tym, czy zwrócona z metody wartość to obiekt ustawiony w mocku;
  • test sprawdza czy kompilator działa, np. czy nie rzuci wyjątku gdy podamy int do metody, która przyjmuje int;
  • partial mock - to usuwam natychmiast dla zasady, bo to taki programistyczny transwestyta.
2
Wibowit napisał(a):

Mam nadzieję, że zrozumiecie o co mi chodzi :)

Rozumiemy :)

Tyle tylko że lekkie implementacje to de facto mocki, które mają dokładnie te same wady.
Trzeba pamiętać, że przy zmianie kontraktu trzeba też zmodyfikować mocki (tak samo jak lekką implementację) , i nikt tego nie przeskoczy.
Pytanie, które się nasuwa - czy jeśli ktoś zmienia kontrakt klasy bez zmiany testów - to wina metodologii czy człowieka? IMO jeśli ktoś siada, demoluje klasę i nie zmienia testów to równie dobrze może wpisać skipTests=true w parametrach. Tak samo jak osoba akceptująca te zmiany.

0

Tyle tylko że lekkie implementacje to de facto mocki, które mają dokładnie te same wady.
Trzeba pamiętać, że przy zmianie kontraktu trzeba też zmodyfikować mocki (tak samo jak lekką implementację) , i nikt tego nie przeskoczy.

Lekkie testowe implementacje można testować dokładnie tymi samymi testami co implementacje produkcyjne, bo testowa implementacja zachowuje interfejs produkcyjnej. Nie trzeba więc niczego pilnować, bo testy posypią się od razu i to tam gdzie trzeba.

Pytanie, które się nasuwa - czy jeśli ktoś zmienia kontrakt klasy bez zmiany testów - to wina metodologii czy człowieka? IMO jeśli ktoś siada, demoluje klasę i nie zmienia testów to równie dobrze może wpisać skipTests=true w parametrach. Tak samo jak osoba akceptująca te zmiany.

Dyscyplina się nie skaluje. Jeśli zmieniasz kontrakt metody to musisz sprawdzić wszystkie zamockowane użycia tej metody, by sprawdzić czy nadal są poprawne. W zdecydowanej większości przypadków nic nie trzeba będzie zmieniać. Podczas przeglądu kodu też nie wyłapiesz problemów, bo zamockowane użycia metod są w testach dla zupełnie innych klas niż ta zmieniana.

Brak gwarancji stałego kontraktu metod to jeden z głównych powodów dla których nie pisze się komentarzy do metod. W komentarzu do metody opisuje się jej kontrakt. Przy zmianie kontraktu trzeba zaktualizować komentarz. Wynika z tego, że wszystkie zamockowane użycia metody powinno się sprawdzać właśnie w tych momentach, które wymagałyby aktualizacji dokumentacji metody (przy założeniu, że taka dokumentacja by istniała).

Z drugiej strony piszemy dokumentację np do endpointów HTTP (vide Swagger/ OpenAPI), bo przy nich mamy sporą gwarancję utrzymania kontraktu, więc te końcówki HTTP jest sens mockować.

1 użytkowników online, w tym zalogowanych: 0, gości: 1