Naprawdę niełatwo było mi wystartować z tym postem. Trudność stanowiło znalezienie odpowiedniego przykładu, na którym mógłbym ten artykuł zbudować - przykładu z jednej strony prostego, a z drugiej wiarygodnego. Właśnie z tą wiarygodnością miałem problem. Wszystkie bowiem przykłady, które przyszły mi do głowy, ukazywały problem, który chcę tutaj opisać, w sposób tak brutalny, tak oczywisty, że nie mogłem uwierzyć, że takie rzeczy zdarzają się naprawdę... jak więc mogłem liczyć na to, że czytelnicy tego posta uwierzą?
Zajrzałem więc do repozytorium kodu źródłowego i historii merge requestów i już w pierwszym merge requeście trafiłem na coś odpowiedniego... choć równie niewiarygodnego, jak przykłady które przychodziły mi do głowy. Ten fatalny fragment kodu jednak istniał, a podobnych widziałem już setki.
Problemem, który tutaj poruszę, będą testy, którym... nie można ufać. Jest to bardzo powszechne zjawisko. Tak powszechne, że nie przypominam sobie developera, którego nie przyłapałbym na pisaniu tego typu testów. Tak powszechne, że nawet dzisiaj, kiedy przeglądałem kod nowego członka zespołu (a dodam, że jest nim jeden z najgenialniejszych developerów, z jakim kiedykolwiek pracowałem) musiałem ostatecznie kliknąć reject, gdyż i On nie ustrzegł się tego błędu. Co mam na myśli? Oto przykład.
Przykład
Oto Piotrek wyrzeźbił kod, którego diagram klas wygląda następująco.
Na diagramie widać trzy elementy:
Klasa Document - opisuje obiekty reprezentujące dokumenty; takie dokumenty mają wiele pól, ale nas interesują dwa: registrationDate - czyli dzień rejestracji dokumentu, oraz type, czyli rodzaj dokumentu (np. oferta, potwierdzenie przyjęcia zamówienia).
Klasa DocumentsWebService - opisuje obiekt, do którego zostaną przekierowane żądania HTTP; klasa ta ma wiele metod do obsługi wielu różnych żądań, ale nas interesuje tylko jedna: findBy; metoda ta zwraca dokumenty, których dzień rejestracji oraz typ zgadzają się z wartościami przekazanymi w argumentach do tej metody; metoda przyjmuje trzy argumenty, których znaczenia z pewnością się domyślisz: registrationDateFrom, registrationDateTo oraz docType.
Interfejs DocumentRepository - opisuje repozytorium bazodanowe, czyli obiekt, który potrafi odczytać/zapisać obiekty klasy Document z/do bazy danych; spośród wielu metod znajdujących się w tym interfejsie nas interesuje tylko jedna - findBy; ta metoda zostanie wywołana w klasie DocumentsWebService w metodzie findBy i wykona odpowiednie zapytanie na bazie danych w celu pobrania dokumentów o określonym dniu rejestracji oraz typie.
Przed Piotrkiem stoi teraz niewdzięczne, ale jakże potrzebne zadanie - ma napisać test integracyjny metody findBy z klasy DocumentsWebService.
Zanim jednak zaczniemy śledzić poczynania Piotrka wróćmy jeszcze na chwilę do metody, którą ma przetestować. Co ta metoda ma robić? Oto opis:
Po pierwsze, na starcie ma zweryfikować poprawność argumentów wejściowych (jeżeli zastanawiasz się po co, to przeczytaj Fail Fast... or die! - część pierwszą i kolejne):
registrationDateFrom - nie może reprezentować daty przyszłej (nie ma sensu pytać o dokumenty zarejestrowane w przyszłości... z góry bowiem wiadomo, że takie nie istnieją)
registrationDateTo - (a) nie może być nullem, (b) nie może reprezentować daty przyszłej i (c) nie może reprezentować daty wcześniejszej niż registrationDateFrom
docType - nie może być nullem
Po drugie musi zwrócić obiekty klasy Document, których pole registrationDate jest większe lub równe registrationDateFrom i mniejsze lub równe registrationDateTo, a pole type jest równe docType.
Prościzna... prawda!? Po co to w ogóle testować!? No cóż... jeżeli naprawdę zastanawiasz się po co, to taki oto fragment kodu znalazłem w czasie code review parę dni temu:
private void setKwotaNadplaty(final Money kwotaNadplaty) {
this.kwotaNadplaty = kwotaZaliczen;
}Płakać się chce. Wróćmy jednak do Piotrka...
... siedzi biedaczysko. Nie chce mu się pisać tych testów, ale... lidera ma niemiłosiernego - wie, że testów Mu nie odpuści. Piotrek jest bardzo inteligentnym człowiekiem i naprawdę świetnym programistą. Kiedy pisze kod, ma wszystko czego trzeba: zapał, doświadczenie, zna teorię, ma intuicję i wyobraźnię, a między uszami potężne CPU. Jest idealnym połączeniem inżyniera i artysty... bo jak wiemy programowanie to z jednej strony inżynieria (patrz książka Rogera S. Pressmana Software engineering: a practitioner's approach), a z drugiej sztuka (Donald E. Knuth i jego słynne dzieło The Art of Computer Programming) - już same tytuły mówią wszystko.
Kiedy jednak przychodzi do pisania testów Piotrek przestaje być inżynierem, jest już tylko artystą. Nie ma bowiem żadnego usystematyzowanego podejścia do pisania testów. Tak właśnie jest w tej chwili. Patrzy na metodę findBy, którą ma przetestować i... niczym Vincent van Gogh czy Jan Sebastian Bach, czeka na natchnieniem. Jeszcze chwilka i coś wymyśli. Jeszcze chwilka.... Jeszcze chwilka... Jeszcze.... jest! Przyszło! Powstają pierwsze linijki kodu. Oto one:
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(null, TOMORROW, Type.OFFER));
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(TOMORROW, null, Type.OFFER));
}Piotrek odetchnął z ulgą. Myśli sobie: szybko poszło, całą tę cholerną walidację argumentów mam już za sobą, a najlepsze jest to, że wyrzeźbiłem to w jednym rzucie, bez odrywania rąk od klawiatury! Tak to jest z natchnieniem, kiedy już przyjdzie robota sama idzie.
Niestety jednak kod, który napisał Piotrek jest nic niewart. Przenieśmy się w czasie, w przyszłość o jeden dzień i popatrzmy na Michała, lidera zespołu w którym poci się Piotrek. Michał przegląda kod.
Zaczyna analizować kod od góry, linijka po linijce. Wygląda to mniej więcej tak. Najpierw widzi:
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {W tym momencie myśli sobie: doskonale, Piotrek będzie tutaj testował, czy metoda prawidłowo weryfikuje argumenty wejściowe. Spogląda na kolejną linijkę:
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(null, TOMORROW, Type.OFFER));...i tutaj przeżywa pierwsze rozczarowanie (czytaj furię). Myśli sobie bowiem: co jest tutaj, do ciężkiej cholery, nie tak? Dlaczego przy tym zestawie argumentów findBy ma rzucić wyjątek? Czy dlatego, że pierwszy argument jest nullem, czy może dlatego, że z drugim argumentem jest coś nie tak, a może nie wolno przeszukiwać dokumentów typu oferta? Michał dodaje przy tej linijce zwięzły, acz bardzo treściwy komentarz: WTF! i klika na przycisk reject.
Z kodu, który napisał Piotrek, Michał niczego nie może się dowiedzieć - nie jest to więc samodokumentujący się kod.
Sfrustrowany przechodzi do drugiej linijki. Tutaj dopiero szlag go trafia. Widzi bowiem coś takiego:
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(TOMORROW, null, Type.OFFER));Tutaj również nie wiadomo dlaczego oczekiwany jest wyjątek. Co najgorsze jednak, w tej linijce Piotrek popełnił naprawdę poważny błąd. Ta linijka bowiem niczego nie testuje... no dobra, przesadziłem - coś tam testuje. Nie jest to jednak kod, który daje nam gwarancję, że kiedy zrefactorujemy metodę findBy i uruchomimy ten test, a w wynik ujrzymy zielony kolor, będzie to oznaczało, że metoda findBy wciąż działa prawidłowo.
Otóż zerknijmy na implementację testowanej metody - wygląda tak:
public Set<Document> findBy(final LocalDate registrationDateFrom,
final LocalDate registrationDateTo,
final Document.Type docType) {
// pierwsza walidacja
if (registrationDateFrom != null)
if (registrationDateFrom.isAfter(timeProvider.currentDate()))
throw new IllegalArgumentException();
// druga walidacja
if (registrationDateTo == null)
throw new IllegalArgumentException();Widzisz już na czym polega problem? Podpowiem Ci. Wyobraź sobie, że z metody findBy usunięto pierwszą walidację - czy wówczas uruchamiając test, powyższy assertThrows przejdzie, czy wskaże na błąd?
Niestety nie wskaże błędu. To dlatego, że w wywołaniu metody findBy w drugim argumencie podano nulla, więc druga walidacja wygeneruje IllegalArgumentException, co spowoduje, że powyższy assertThrows dostanie swój wyczekiwany wyjątek i problemu nie wychwyci.
A teraz wyobraź sobie, że w findBy pozostawiono pierwszą walidację, ale w wyniku refactoringu usunięto, przez pomyłkę, drugą - czy teraz assertThrows wskaże błąd?
Niestety, o zgrozo, również nie! To dlatego, że w pierwszym argumencie wywołania metody findBy podano datę z przyszłości (TOMORROW), a to spowoduje, że pierwsza walidacja wygeneruje IllegalArgumentException. Ileż więc wart jest ten test, który wciąż nie chroni developerów przed błędami regresji? Jest nic nie warty. Nie można bowiem na nim polegać. To, że zakończy się zielonym, nic nie znaczy.
Absurdalne, czyż nie?! Ciężko w ogóle uwierzyć, że doświadczony developer mógł wyrzeźbić taki test. A jednak, tak właśnie było - jest to kod, z życia wzięty. Ja go nie wymyśliłem. I dodam, że widuję takie kwiatki bardzo często u nowych developerów w moim zespole, bez względu na to, czy są to juniorzy, czy seniorzy.
Czarę goryczy u Michała przelewa brak testów walidacji trzeciego argumentu: docType. To chyba nie wymaga już żadnego komentarza. Prawda? Jeżeli faktycznie Piotrek pisał te testy pod natchnieniem, to musiało ono przypominać natchnienia, których doświadczał Witkacy malując portrety, na których później pisał T.C + Co.
No dobra, ale jak można było to zrobić lepiej?
Zasada najmniejszych różnic
Powyższego problemu można było z łatwością uniknąć stosując coś, co nazywam zasadą najmniejszych różnic. Brzmi ona następująco:
Każdy pojedynczy test, różni się od testu wzorcowego w najmniejszy możliwy sposób.
Co to znaczy w praktyce? Wyobraźmy sobie, że to my piszemy test metody findBy. Moglibyśmy zacząć go od napisania czegoś takiego:
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {
assertDoesNotThrow(
() -> documentsWebService.findBy(YESTERDAY, TODAY, Type.OFFER));
// to be continued...Ta pierwszy linijka to jest właśnie test wzorcowy. Mówi nam, że dla takich oto trzech argumentów wyjątek nie ma prawa zostać rzucony.
Teraz zaczniemy pisać kolejne testy, robiąc najmniejsze możliwe różnice względem powyższego testu wzorcowego. Żeby jednak nasze testy miały jakiś logiczny porządek, zastosujemy drugą podstawową zasadę, która przydaje się przy pisaniu testów. Brzmi ona następująco:
Testy piszemy od lewej do prawej.
Co to znaczy? To znaczy, że zajmiemy się teraz pierwszym od lewej argumentem. W ogóle nie będziemy dotykać kolejnych argumentów aż do czasu, kiedy stwierdzimy, że nie mamy już więcej sensownych pomysłów jak można by popastwić się nad pierwszym argumentem.
Zadajemy więc sobie bardzo konkretne pytanie: jakich wartości nie wolno przekazać w argumencie pierwszym? Prawda, że to pytanie jest znacznie bardziej konkretne niż oczekiwanie na natchnienie?
Po chwili namysłu wpadamy na następujący pomysł: data z przyszłości jest nieakceptowalna - dopisujemy więc taki oto test.
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {
assertDoesNotThrow(
() -> documentsWebService.findBy(YESTERDAY, TODAY, Type.OFFER));
// walidacja argumentu pierwszego
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(TOMORROW, TODAY, Type.OFFER));
// to be continued...Jak widzisz w tym teście wywołanie metody findBy różni się tylko jednym argumentem od wywołania tej metody w teście wzorcowym - to właśnie jest zachowanie zasady najmniejszych różnic.
Myśląc o niedozwolonych wartościach pierwszego argumentu zastanawiałeś się nad nullem - czy jest dozwolony, czy nie. Ostatecznie stwierdziłeś, że jest. Uznałeś jednak, że nie było to oczywiste, więc postanowiłeś udokumentować to dla tych, co przyjdą po Tobie - w teście dodałeś więc kolejną asercję. Teraz metoda testowa wygląda następująco:
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {
assertDoesNotThrow(
() -> documentsWebService.findBy(YESTERDAY, TODAY, Type.OFFER));
// walidacja argumentu pierwszego
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(TOMORROW, TODAY, Type.OFFER));
assertDoesNotThrow(
() -> documentsWebService.findBy(null, TODAY, Type.OFFER));
// to be continued...I oto doszedłeś do wniosku, że nie masz już więcej pomysłów jakie wartości pierwszego argumentu są niedozwolone. Zgodnie z zasadą: od lewej, do prawej, przechodzisz więc do pastwienia się nad drugim argumentem metody findBy.
Zadajesz sobie znów bardzo konkretne pytanie: jakich wartości nie wolno przekazać w argumencie drugim? Myślisz i już po chwili dochodzisz do wniosku, że: (a) null nie jest dozwolony, (b) wartość z przyszłości oraz (c) wartość wcześniejsza niż ta, podana w pierwszym argumencie. Tak więc dopisujesz kolejne trzy testy.
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {
assertDoesNotThrow(
() -> documentsWebService.findBy(YESTERDAY, TODAY, Type.OFFER));
// walidacja argumentu pierwszego
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(TOMORROW, TODAY, Type.OFFER));
assertDoesNotThrow(
() -> documentsWebService.findBy(null, TODAY, Type.OFFER));
// walidacja drugiego argumentu
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, null, Type.OFFER));
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, TOMORROW, Type.OFFER));
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, DAY_BEFORE_YESTERDAY,
Type.OFFER));
// to be continued...Jak widzisz, dodaliśmy trzy nowe testy, ale każdy z nich zmienił tylko i wyłącznie (!) drugi argument. Wartości argumentów pierwszego i trzeciego są identyczne jak w teście wzorcowym, czyli w pierwszym assertDoesNotThrow - to właśnie jest zasada najmniejszych różnic.
OK, czas przejść do argumentu trzeciego. Znów zadajemy sobie znane nam już bardzo konkretne pytanie. Odpowiedź na nie brzmi: null nie jest dozwolony. Tak więc dopisujemy ostatni już test. Oto jak wygląda ostateczna (no.... prawie ostateczna) wersja metody:
@DisplayName("Walidacja argumentów wejściowych")
@Test
public void findBy_01() {
assertDoesNotThrow(
() -> documentsWebService.findBy(YESTERDAY, TODAY, Type.OFFER));
// walidacja argumentu pierwszego
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(TOMORROW, TODAY, Type.OFFER));
assertDoesNotThrow(
() -> documentsWebService.findBy(null, TODAY, Type.OFFER));
// walidacja drugiego argumentu
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, null, Type.OFFER));
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, TOMORROW, Type.OFFER));
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, DAY_BEFORE_YESTERDAY, Type.OFFER));
// walidacja trzeciego argumentu
assertThrows(IllegalArgumentException.class,
() -> documentsWebService.findBy(YESTERDAY, TODAY, null));
}Podsumowanie
Nasza metoda testująca jest gotowa. Dzięki zastosowaniu zasady od lewej do prawej, testuje walidację każdego (!) argumentu wejściowe, a dzięki zastosowaniu zasady najmniejszych różnic, nie ma ryzyka, że zmiany w kodzie metody findBy zostaną niezauważone.
Naszą metodę testową wciąż jednak można oszukać. Nie ma to jednak nic wspólnego z dwiema wyżej wspomnianymi zasadami, ale po prostu z tym, że w niedokładny sposób testujemy porównania. O co chodzi?... wyjaśnię w kolejnym artykule.
W tym miejscu muszę się zatrzymać, gdyż rozmiar artykułu wynosi około osiem minut czytania - a powszechnie wiadomym jest, że współczesny, dorosły homo sapiens nie potrafi skupić się na dłużej. W kolejnej części tego tematu (dostępnej tutaj), przedstawię jeszcze jeden przykład zastosowania dwóch w/w zasad, żeby utwierdzić Cię w przekonaniu, jak wielka jest ich wartość i jak prosto pisze się według nich testy. A więc... do kolejnego spotkania!
"One more thing..."
A może masz ochotę na naprawdę mocne szkolenie?
W styczniu, został uruchomiony produkcyjnie największy system księgowy w Polsce. Rocznie przechodzi przez niego 330 miliardów złotych (!), co dzień korzysta z niego 30 tysięcy (!) użytkowników, realizuje milion (!) księgowań na godzinę (!), a główna tabela księgowa zawiera 8 miliardów(!) rekordów. To moje dziecko, to ja go zaprojektowałem. W czasie szkolenia przedstawię Ci architekturę tego systemu oraz różne techniki, które z powodzeniem stosuję od 16 lat. Jeżeli jesteś zainteresowany, to zajrzyj... tutaj.
Comments