Unit testy - czy odwzorowywać "produkcyjną" konfigurację?

Cześć,

mam kilka klas walidatorów, dla uproszczenia nazwijmy je ValidatorA i ValidatorB. Każdy z nich jest fajnie otestowany.
Z drugiej strony mam serwis przyjmujący w konstruktorze listę walidatorów. Na produkcji jest to ValidatorA i ValidatorB. Zastanawiam się jak powinny wyglądać testy takiego serwisu. Czy w testach powinienem korzystać z new Service([new ValidatorA(), new ValidatorB()])? Czy np. nie tworzyć takiego powiązania, mieć większą elastyczność i testować tylko "jednostkowo" klasy? Wyobrażam sobie, że w sytuacji w której dochodzi nowy walidator druga opcja jest wygodniejsza.
W pewnym momencie doszedłem nawet do tego, że może nie potrzebuję testować bezpośrednio samych walidatorów, a tylko zachowanie serwisu.
Jakieś dobre praktyki, przykłady?

Odpowiedz sobie na pytanie - jak ten serwis powinien działać: czy walidacja to jest to co od niego wymagasz? czy też jakaś opcjonalna, konfigurowalna rzecz.
Zwykle najwięcej sensu ma testowanie "jednostkowo" całego Serwisu razem z walidatorami. Zwykle nowy walidator przyjdzie jak zmienią się wymagania do Serwisu i dopiszesz nowe testy do serwisu.

jarekr000000 napisał(a):

Odpowiedz sobie na pytanie - jak ten serwis powinien działać: czy walidacja to jest to co od niego wymagasz? czy też jakaś opcjonalna, konfigurowalna rzecz.

To jest coś czego od niego wymagam. Z drugiej strony te walidacje powtarzają się w różnej konfiguracji w innych miejscach - załóżmy, że trzy serwisy mają wpięte takie walidatory w różnych kombinacjach/kolejnościach. Zastanawiam się czy testowanie każdego nie dołoży mi pracy i powtórzeń w testach (3x sprawdzenie tej samej walidacji, ale w różnych miejscach). Z drugiej strony zapewni mi, że te serwisy będą na pewno wykonywały walidację.

Jeśli miałbym testować "wymagania serwisu" czy warto dodatkowo testować jednostkowo walidatory? Załóżmy, że są proste i nie byłbym w stanie napisać "dokładniejszych" testów niż to co przetestowałem wyżej.

Zwykle API Serwisów jest relatywnie ważne i powinno być dobrze przetestowane. Jak masz powtórzenia to da się to ogarnąć - np. testami parametrycznymi, gdzie parametry to serwisy.

Co do testowania walidatorów - im ciekawszy jest tam kod, w im więcej miejscach używasz tym większy sens ma ich osobne testowanie. Choćby po to, żeby szybciej wiedzieć, że ewentualna zwałka polegająca na tym, że wysypują się wszystkie serwisy jest tak naprawdę w walidatorze. Ale to już decyzja - jak są naprawdę proste i trudno się pomylić to możesz odpuścić - będą i tak przy okazji serwisów przetestowane.

załóżmy, że trzy serwisy mają wpięte takie walidatory w różnych kombinacjach/kolejnościach.

No dobra, ale kluczowe dla ciebie jest, że serwis X robi walidacje Y, niezależnie od tego czy ta walidacja była już gdzieś indziej testowana. Nie musisz robić jakichś testów dla przypadków brzegowych danego walidatora testując serwis, ale sam test który potwierdza że walidator tam jest musi jak najbardziej być. Wyobraź sobie że masz 2 serwisy, jeden pozwala usuwać użytkowników a drugi ich banować. Oba mają walidator który sprawdza czy request przyszedł od admina. Zgodzisz się chyba ze warto byłoby sprawdzić w obu przypadkach role użytkownika? ;)

Idąc dalej: moim zdaniem najprościej wyjść od wymagań systemu. Wymagania to będzie coś w stylu np.:

• system zwraca 401 jeśli użytkownik nie jest zalogowany
• system zwraca 403 jeśli użytkownik nie jest adminem i nie może banować użytkowników
• system zwraca 200 i nakłada bana jeśli użytkownik jest adminem

I dla takich sytuacji należy napisać testy, niezależnie od tego co testujesz w innym miejscu.

Wszystko sprowadza się do jednej kwestii - czy można zmienić listę walidatorów bez zmiany kodu klasy?

Jeśli masz coś takiego:

public class MyService {
    private final List<Validator<MyRequest>> validators;
    
    public MyService(final List<Validator> validators) {
        this.validators = validators;
    }
    
    public void handle(final MyRequest request) {
        validators.forEach(v -> v.validate(request));
        
        // ... do something else
    }
}

to:

• test jednostkowy nie jest w stanie sprawdzić poprawności walidacji (test jednostkowy powinien sprawdzać tylko tyle, co się stanie gdy jeden z walidatorów rzuci błędem)
• test sprawdzający poprawność reguł walidacji testem jednostkowym nie będzie

Oczywiście można na takie coś przymknąć oko. Taki test nie-do-końca jednostkowy będzie OK w zdecydowanej większości przypadków, tzn. wtedy gdy nie będziesz miał dwóch instancji MyService o różnej ilości walidatorów na produkcji, tj.

public class MyServiceFactory {
    public MyService createFirstInstance() {
        List<Validator<MyRequest>> validators = // ... create validators for UC #1
        
        return new MyService(validators);
    }
    
    public MyService createSecondInstance() {
        List<Validator<MyRequest>> validators = // ... create validators for UC #2

        return new MyService(validators);
    }
}

Wtedy lepiej przetestować walidatory oddzielnie, a przy testach MyService jedynie się upewnić, że wszystkie walidatory są wołane i co się stanie gdy któryś z nich rzuci błędem.

Wtedy lepiej przetestować walidatory oddzielnie, a przy testach MyService jedynie się upewnić, że wszystkie walidatory są wołane i co się stanie gdy któryś z nich rzuci błędem.

Ja bym właśnie tak nie robił. Dodanie nowej reguły walidacji powoduje konieczność dopisania testu walidatora i testu na jego uzycie w serwisie. Drugi test jest 10x istotniejszy i - o ile nie powoduje to eksplozji przypadków testowych - testowałbym najgłębiej na poziomie serwisu/fasady. Nieocenionym benefitem takiego podejścia jest to, że potem możesz sobie dowolnie refaktorować, wydzielać, łączyć, chainować te walidatory, a testy pozostaną nieruszone - i tak to się powinno robić.

Test jednostkowy nie oznacza z definicji testu klasy. Od Ciebie jako programisty należy takie określenie czym jest ta jednostka, aby kod zachował najważniejsze dla Ciebie parametry, np. łatwą rozszerzalność.

@Charles_Ray:

Dodanie nowej reguły walidacji powoduje konieczność dopisania testu walidatora i testu na jego uzycie w serwisie.

Nie do końca. Możesz przetestować czy serwis wykorzystuje wszystkie walidatory przekazane jako argument, a potem sprawdzać każdy walidator z osobna. Właśnie takie podejście gwarantuje ci niezależność działania testów klasy MyService

Od Ciebie jako programisty należy takie określenie czym jest ta jednostka, aby kod zachował najważniejsze dla Ciebie parametry, np. łatwą rozszerzalność.

Właśnie o tym pisałem. Jeśli w trakcie kompilacji:

• istnieje jeden i tylko jeden znany zestaw walidatorów z którego będzie korzystał MyService to parę można spokojnie testować razem. Ba, jeśli walidator jakiś walidator wykorzystywany jest tylko i wyłącznie w MyService to nie musisz go testować oddzielnie - wystarczy, że MyService się wywali w przypadku jego nieprawidłowego działania a i tak będziesz wiedział o co chodzi
• istnieje mała, ustalona liczba zestawów walidatorów z których będzie korzystał MyService to wypada MyService przetestować oddzielnie oraz dla każdego zestawu walidatorów oddzielnie
• ta liczba walidatorów jest duża lub tworzona w runtime to nie ma co próbować przetestować wszystkiego - wtedy testujesz tylko czy MyService korzysta z podanych walidatorów i tyle

@Charles_Ray: tak, można użyć mockowania przy czym:

• mockowanie zależności - wbrew obiegowej modzie - nie jest złe
• nie musisz nawet używać żadnych narzędzi - wystarczy przekazać testowy walidator

Mockowanie zależności jest zle, ponieważ betonujesz w testach api klas uniemożliwiając swobodny refactoring. Testujmy zachowania i wymagania biznesowe, a nie implementację - powtarzane 1000 razy.

@Charles_Ray: to "powtarzane tysiąc razy" zdanie absolutnie nic nie znaczy - ale to temat na inną dyskusję i nie zamierzam go ciągnąć.

Wróćmy do naszego przypadku - mając tak zdefiniowaną klasę:

public class MyService {
    private final List<Validator<MyRequest>> validators;
    public MyService(final List<Validator> validators) {
        this.validators = validators;
    }

    public void validateAndHandle(final MyRequest request) {
        // validate and handle
    }
}

Istnieją dwa przeciwstawne podejścia:

1. Testować reguły biznesowe
2. Przetestować zachowanie klasy, tj. upewnić się, że wszystkie walidatory zostały wywołane oraz że wysypanie się przynajmniej jednego spowoduje rzucenie błędem.

Podejście #1 sprawdzi się gdy masz ograniczoną liczbę zestawów walidatorów.
Podejście #2 sprawdzi się gdy taka liczba zestawów walidatorów jest bardzo duża - np. ekstremalny przypadek - użytkownik może sobie je wyklikać. W takim przypadku nie jesteś w stanie przetestować reguł biznesowych - bo ich po prostu nie znasz.

Podejście #1 sprawdzi się gdy masz ograniczoną liczbę zestawów walidatorów.
Podejście #2 sprawdzi się gdy taka liczba zestawów walidatorów jest bardzo duża - np. ekstremalny przypadek - użytkownik może sobie je wyklikać. W takim przypadku nie jesteś w stanie przetestować reguł biznesowych - bo ich po prostu nie znasz.

Nawet w drugim przypadku mogę testować przez fasadę, nie muszę robić żadnych jawnych założeń co do implementacji pod spodem. Robię sobie testowy walidator, który po odpaleniu zmienia stan, który assertuję w teście - wow, bez mocków.

@wartek01 testuj jak chcesz ale weź pod uwagę że:

1. Klienta interesuje czy system działa, a nie czy w klasie masz listę walidatorów czy 3 hardkodowane, czy siedzi hindus i waliduje ręcznie
2. Dziś masz tam listę a jutro dwa z tych walidatorów złożą się w 1 a trzeci rozbije na 2 i będziesz przepisywać swoje testy, mimo że wszystko działa
3. Testuj zachowanie a nie kod.O tym czy kod się zmienił wystarczy ze informuje cię git, nie potrzeba do tego jeszcze testów.

Różne są podejścia. Ja lubie jak puszczam testy i wiem ze jak są zielone to znaczy ze system działa i można mergować. Bardzo nie lubie kiedy refaktor strukturalny sprawia ze testy zaczynają failować.

@Charles_Ray: po pierwsze - o tym pisałem. Po drugie - testowy walidator to też mock.

@Shalom: zawsze wychodzi się od potrzeb klienta, ale istnieje też coś takiego jak utrzymywalność systemu - dlatego staramy się właśnie nie używać magicznych zmiennych i pilnujemy by kod był czytelny.

Dziś masz tam listę a jutro dwa z tych walidatorów złożą się w 1 a trzeci rozbije na 2 i będziesz przepisywać swoje testy, mimo że wszystko działa

Mam bardzo mocne wrażenie, że nie do końca się rozumiemy. Zawsze istnieje ryzyko, że zmiana wymagań biznesowych pociągnie za sobą zmianę kodu oraz zmianę testów. Testy są po to, żeby przy mniej inwazyjnych zmianach wyłapywać potencjalne problemy - bo choćbyś stanął na głowie to jeśli zmieni ci się kontrakt na MyService.handle to będziesz musiał zmienić i implementację, i testy.

I powtórzę się - wszystko zależy od konkretnego przypadku. W przykładzie podanym przeze mnie podejście do testów klasy MyService zależy od kontekstu w jakim ta klasa występuje - a konkretniej od tego, skąd brane są zestawy walidatorów. Inaczej podejdziesz do testowania tego przy typowym CRUDzie, gdzie wiesz jaki jest ten zestaw i będzie jeden na całą aplikację - a inaczej podejdziesz gdy ten zestaw będzie konfigurowalny przez użytkownika.

Testuj zachowanie a nie kod.O tym czy kod się zmienił wystarczy ze informuje cię git, nie potrzeba do tego jeszcze testów.

"testuj zachowanie, a nie kod" to dla mnie taki niewiele wnoszący slogan. Zrozumiałbym "testuj zachowanie, a nie zależności", ale "testuj zachowanie, a nie kod" czy "testuj zachowanie, a nie implementację" nie mają sensu - bo czym jest jakikolwiek test jeśli nie sprawdzaniem jak zachowa się konkretna implementacja - czyli konkretny kawałek kodu?

Rozumiem o co chodzi - o to, żeby zwracać uwagę na "kontraktowe" wyniki działania kodu, tj. dowolny kod może sobie tam pod spodem układać w pamięci obliczenia jak chce (dopóki oczywiście to nas nie obchodzi) - byleby to robił zgodnie z wymaganiami.

Ja lubie jak puszczam testy i wiem ze jak są zielone to znaczy ze system działa i można mergować. Bardzo nie lubie kiedy refaktor strukturalny sprawia ze testy zaczynają failować.
Testowanie zachowania jest testowaniem kodu.

No fajnie, ale ja nie o tym.

ale istnieje też coś takiego jak utrzymywalność systemu

Jak najbardziej. A pisanie testów "implementacji" tylko tą utrzymywalność obniża, bo cementuje ci kod albo zmusza do bezmyślnego poprawiania testów, które psuje każde dotkniecie kodu, bo zmieniłeś kolejność walidatorów na swojej liscie i nagle twoje testy są czerwone ;)

Zawsze istnieje ryzyko, że zmiana wymagań biznesowych

Nikt tu nie mówi o zmianie wymagań. Z definicji zmiana wymagań = konieczność zmiany testów. Jak zmieniły się wymagania a ty nie musisz poprawiać testów, to znaczy ze guzik te testy sprawdzały ;)
Mówimy o sytuacji gdzie wymagania są te same, a zmienia się implementacja. W takiej sytuacji testy powinny być bez zmian bo testują zachowanie. Jeśli zrobiłeś refaktor strukturalny i musisz poprawiać testy to trzeba sobie zadać pytanie co te testy faktycznie sprawdzają?.

Nie twierdzę ze nie możesz sobie zrobić unit testów poszczególnych walidatorów albo testów tego swojego composite validator złożonego z kilku. To akurat całkiem dobre miejsce na unit testy. Twierdzę jedynie że kluczowe testy to te związane z wymaganiami i od nich należy wyjść. Pozostałe są nice to have.

bo czym jest jakikolwiek test jeśli nie sprawdzaniem jak zachowa się konkretna implementacja - czyli konkretny kawałek kodu?

Różnica jak między piciem w Szczawnicy a szczaniem w piwnicy. Zupełnie czym innym jest sprawdzenie czy kod robi to co klient chce żeby robił a zupełnie czym innym jest testowanie czy kod działa tak jak programista myśli ze działa. To jest mniej więcej taka różnica jak między testowaniem blackbox i whitebox. Jedno skupia się na tym czy na zadany input otrzymujesz oczekiwany output, a drugie skupia się na analizie przepływu sterowania w testowanym kodzie.

@Shalom:

Shalom napisał(a):

ale istnieje też coś takiego jak utrzymywalność systemu

Jak najbardziej. A pisanie testów "implementacji" tylko tą utrzymywalność obniża, bo cementuje ci kod albo zmusza do bezmyślnego poprawiania testów, które psuje każde dotkniecie kodu, bo zmieniłeś kolejność walidatorów na swojej liscie i nagle twoje testy są czerwone ;)

Na 99% różnimy się nomenklaturą. Dla mnie takie coś:

interface SummingService {
    int sum(final int a, final int b);
}

class ConcreteSummingService implements SummingService {
    
    @Override
    public int sum(int a, int b) {
        return (a+a+b+b)/2;
    }
}

class SummingServiceTests {
    
    public void test() {
        SummingService service = new ConcreteSummingService();
        
        if (service.sum(1, 2) != 3) {
            // fail
        }
    }
}

jest dla mnie po prostu testem implementacji ConcreteSummingService i tyle. Nie musisz zaglądać do środka, żeby sprawdzić jak to działa.

Nie twierdzę ze nie możesz sobie zrobić unit testów poszczególnych walidatorów albo testów tego swojego composite validator złożonego z kilku. To akurat całkiem dobre miejsce na unit testy. Twierdzę jedynie że kluczowe testy to te związane z wymaganiami i od nich należy wyjść.

No i właśnie wyszedłem od wymagań. Jasno postawiłem sprawę, że to jak powinny wyglądać testy MyService zależy od tego w jakim kontekście występuje na produkcji.

Różnica jak między piciem w Szczawnicy a szczaniem w piwnicy. Zupełnie czym innym jest sprawdzenie czy kod robi to co klient chce żeby robił a zupełnie czym innym jest testowanie czy kod działa tak jak programista myśli ze działa.

Zgadzam się. Tyle tylko, że pisząc dowolny test piszesz test kodu/implementacji. Testy, które nie testują żadnego kodu nie mają sensu.

wartek01 napisał(a):

Zgadzam się. Tyle tylko, że pisząc dowolny test piszesz test kodu/implementacji. Testy, które nie testują żadnego kodu nie mają sensu.

Testy testują kod, owszem. Ale piszemy testy nie po to, żeby testować kod, lecz zachowanie (bez względu na poziom testów). Jeśli rzeczywiście tylko "różnimy się nomenklaturą", to @wartek01 ma złą nomenklaturę, sorki...

Tyle tylko, że pisząc dowolny test piszesz test kodu/implementacji.

Jeśli zrobie test który wysyła request HTTP do systemu i sprawdza czy zwrócony wynik zgadza się z tym co klient podał w wymaganiach, to testuje czy system działa. Na twoim przykładzie, załóżmy że jest endpoint który przyjmuje listę intów i zwraca ich sumę. Mogę sobie puścić losowe testy albo testy parametryzowane, które będą wysyłać te listy i sprawdzać wyniki. Zauważ ze taki test w ogóle nie zajmuje się tym jak to sumowanie jest robione. Czy tam jest jakiś SummingService czy może jest to rozbite na 17 klas, czy może są różne implementacje w zależności od rozmiaru danych wejściowych, albo rozmiaru argumentów, czy w ogóle siedzi tam hindus i sumuje na kalkulatorze a przeklepuje wynik (wiec żadnego "kodu" tam nie ma :D )

To co pokazałeś jako SummingServiceTests to jest test kawałka kodu. Jeszcze raz: nie mówie że to źle, bo to akurat ok miejsce na zrobienie unit testu, ale twój SummingServiceTests wcale nie oznacza że funkcja systemu system sumuje podane przez użytkownika liczby działa. Ten kawałek kodu może np. w ogóle nie być nigdzie podpięty. Dlatego kluczowy jest test funkcji systemu / zachowania, a dopiero jak masz takie testy to możesz się bawić w szczegółowe testowanie kawałków kodu, które uważasz że tego wymagają.

Na przykładzie tych twoich walidatorów, kluczowym testem jest sprawdzenie czy serwis odrzuca niepoprawne requesty, niezależnie od tego jak zaimplementowałeś walidacje. Czy masz tam wszystko ifami w jednej funkcji, czy masz porobione osobne klasy czy jeszcze jakoś inaczej. Cóż ci po tych testach jednostkowych walidatorów, skoro zapomnisz dodać walidator IsAdminUser do listy w tym serwisie i okaże sie że wszystkie twoje testy tego walidatora są zielone, a serwis pozwala każdemu wykonywać akcje admina? ;)

Tak mi przyszło do głowy: Testowanie powinno być trochę tak zrobione, jak eksperyment chińskiego pokoju -- nie ważne co jest w środku, ważne, że pokój (jako całość) rozumie chiński. :)

@Shalom @koszalek-opalek : przypominam, że ciągle piszę o testach jednostkowych. Testy jednostkowe z definicji testują kod.

wartek01 napisał(a):

@Shalom @koszalek-opalek : przypominam, że ciągle piszę o testach jednostkowych. Testy jednostkowe z definicji testują kod.

Nie. Testy jednostkowe z definicji testują jednostki (coraz bardziej, nie wiem, czy się rozumiemy). Do śmieci nadają się wszystkie testy "jednostkowe", które testują implementację. Jednostką, którą testujemy jest np. jakiś interfejs, nie kod, który jest pod spodem...

Ja myśle że jednak testują jednostkę czymkolwiek by ona nie była ;) Pytałeś o testowanie serwisu, a to robi się testując jego kontrakt. Tzn wołasz ten serwis i sprawdzasz czy zwraca oczekiwane wyniki. Jeszcze raz: w ogóle nie powinno cię przy tym obchodzić jak ten serwis realizuje swoje funkcje. Serio, to czy on w środku ma listę walidatorów, czy wszystko jest ifami w jednej funkcji w ogóle nie powinno być "widoczne" z poziomu testów, bo będziesz te testy przepisywać do śmierci. Kluczowe jest co? a nie jak?.

Jak to mawiał klasyk,

Zawsze się tam trochę mockuje.

@koszalek-opalek @Shalom:

Nie. Testy jednostkowe z definicji testują jednostki (coraz bardziej, nie wiem, czy się rozumiemy). Do śmieci nadają się wszystkie testy "jednostkowe", które testują implementację. Jednostką, którą testujemy jest np. jakiś interfejs, nie kod, który jest pod spodem...

Pytałeś o testowanie serwisu, a to robi się testując jego kontrakt.

Napiszę po raz ostatni i przestaję się tutaj odzywać:
Jeśli macie interfejs MyService oraz jego implementację - MyConcreteService:

public interface MyService {

    /* Should do something */
    void doSomething();
}

public class MyConcreteService implements MyService {
    /* ... */
}

i w teście jednostkowym jako implementację interfejsu MyService podajecie MyConcreteService:

public class MyTests {
    
    @Test
    public void myTest() {
         MyService service = new MyConcreteService();
         // assertions etc.
     }
}

to testowaniu podlega implementacja MyConcreteService. Nie da się "przetestować kontraktu" bez testowania implementacji.

@wartek01: Nie ma się co unosić. Ale ja bym zrobił test parametryczny z parametrem MyService service i miałbym test napisany dla interfejsu, nie dla implementacji. :)

Ja zawsze rozróżniam dwa typy testów jednostkowych zorientowane na stan i behawioralność jednostki. Te zorientowane na stan to testujące kontrakt "dam Ci takie argumenty daj mi taką odpowiedź". I wtedy nie jest ważna implementacja.

Operation op = new Multiplication();
op.do(1, 3) // 3

Operation op = new Addition()
op.do(1, 3) // 4

Implementacja tutaj jest ważna tylko po to, żeby znać wynik. Nie zawsze można zrealizować testy zorientowane na stan, zależy od kodu i programisty :P

Przy behawioralnych już musimy wejść w implementacje, wtedy sprawdzamy czy w danym momencie, funkcja miała jakiś side effect. Powinno się tworzyć tak kod, żeby było jak najwięcej zorientowanych na stan.

Więc to jest tak, żeby dążyć do tego, aby zredukować znaczenie implementacji w testach

@m94: Niby tak -- ale behawioralne też nie testują implementacji tylko zachowanie (jak wskazuje nazwa), czyli też pewnego rodzaju kontrakt -- tylko brzydki.

Powinno się tworzyć tak kod, żeby było jak najwięcej zorientowanych na stan.

No tylko jak czytam wypowiedź to akurat opisujesz medody bezstanowe ("dam Ci takie argumenty daj mi taką odpowiedź")

Poza tym side effecty da się testować (prawie) dokładnie tak samo jak normalne rezultaty - tylko trzeba je zwracać.

nie opisuje metod bezstanowych, tylko testy, a testy zorientowane na stan to np. W przypadku op.do(1, 3) dla dodawania stanem jest warość numeryczna 4, w przypadku funkcji zwracającej zapisaną encje, będzie to ta encja. Pojęcia te są ściśle ze sobą związane, ale, że w obu występuje słowo stan nie oznacza, że to to samo. Jak chcesz przetestować side effect System.out.println w javie jak stan ? Co zwrócisz ? Jak zrobisz z tego np. IOMonad to już nie będzie side effect, a struktura zwrócona nawet w monadzie to nie test