danek
2019-07-10 12:43

Nie wiem czy to jest jakiś duży problem, ale mam wrażenie, że ludzie pisząc testy, zapominają o tym, że można wydzielać metody. O co mi chodzi?
Prosty przykład:
Fragment testu sprawdzający czy po zaplanowaniu pobraniu pewnych danych, dane faktyczie są pobrane. Takich testów jest kilka

    system.setExternalSourceMatchList(List.of(new MatchInfo("host", "guest", 1, false, MatchResult.NOT_SET, system.now()))); 
    system.advanceTimeBy(3, TimeUnit.HOURS);
    system.setExternalSourceMatchList(List.of(new MatchInfo("host", "guest", 1, true, MatchResult.DRAW, matchStartDate)));
    system.advanceTimeBy(10, TimeUnit.MINUTES);

Zamiast tego, przygotowując sobie dwie proste metody (aby można było użyć tego w kilku testach)

    private MatchInfo planned() {
        matchStartDate = system.now();
        return new MatchInfo("host", "guest", 1, false, MatchResult.NOT_SET, matchStartDate);
    }

    private MatchInfo finished() {
        return new MatchInfo("host", "guest", 1, true, MatchResult.DRAW, matchStartDate);
    }

Całośc dla osoby czytającej całość po raz pierwszy wygląda prościej

    system.setExternalSourceMatchList(List.of(planned()));
    system.advanceTimeBy(3, TimeUnit.HOURS);
    system.setExternalSourceMatchList(List.of(finished()));
    system.advanceTimeBy(10, TimeUnit.MINUTES);

Jeśli dane wejściowe są bardziej złożone, to można iść nawet krok dalej i przygotować sobie specjalne metody pod budowanie odpowiednich danych.
Mieliśmy raz projekt na zaliczenie który analizował scenariusze. Scenariusz składał się z pewnych metadanych i opisu kolejnych kroków, gdzie krok mógł mieć podkroki. Stworzyliśmy buildera i dzięki naturalnym wcięciom używamym w javie już sam kod odzwierciedlał strukture danych:

    ScenarioDTO scenarioDTO = new ScenarioBuilder()
                .addSteps(
                        step("",
                                step(""),
                                step(""),
                                step("",
                                        step(""))),
                        step("")
                )
                .build();

Dzięki temu nie trzeba się zagłębiać co jest podkrokiem czego, bo naturalnie było widać.

Więc wydaje mi się, że warto czasem trochę dopracować kod testów, żeby w przyszłości łatwiej je było czytać

xfin

A po co komu czytać testy - widzisz zielone to jest dobrze, widzisz czerwone to jest źle :D

tdudzik

Do poprzedniego projektu udało mi sie wprowadzic takie udogodnienia, mam na myśli buildery właśnie. Ktoś nieznający domeny sporo mógł się dowiedzieć z samych testów. W tym temacie polecam "Growing Object Oriented Software Guided by Tests" :)

kelog

Niestety, w wielu projektach nie inwestuje się czasu w jakieś utile generujące gotowe dane. Zamiast tego mamy ścianę new set set set set new set set set. Zwykle skopiowaną kilka razy :P

Potat0x

A gdy potrzebne są przykłady poprawnych DTO nie tylko w testach, ale też w produkcyjnej części kodu - jak ładnie do tego podejść? Stworzyć osobną klasę ValidMatchInfoDtoGenerator? Dodać statyczną metodę zwracającą przykładowe DTO do MatchInfoBuildera? Od pierwszego sposobu odrzuca mnie namnożenie klas, a od drugiego łamanie SRP i zaśmiecanie buildera.

danek

W testach podaje zawsze poprawnego DTOsa, chyba, że testuje gdzieś walidacje. Co do samego sprawdzania danych wejściowych to jak najbardziej mam osobne klasy od tego. Mnożenie klas to nic złego ;)

Potat0x

Jest metoda zwracająca RequestDtoBuilder w pełni zainicjalizowany zahardkodowanymi, poprawnymi wartościami. Wystarczy .build() i mamy prawidłowy RequestDto. Potrzebuję tego w testach - do łatwiejszego testowania walidatorów i kontrolerów, i na produkcji - żeby dorzucić przykład poprawnego RequestDto do odpowiedzi 422. Zastanawiam się gdzie umieścić taką metodę :)

tdudzik

Zduplikować. A little copying is better than a little dependency. :)

Potat0x

Takiego rozwiązania się nie spodziewałem :P

tdudzik

Czasami najprostsze rozwiązania są najlepsze. :)

no_solution_found

problemem jest wg mnie to, że testy traktuje się jako "gorszą" część projektu, gdzie wzorce projektowe i clean code już tak potrzebny nie jest.

Kermii

Dla mnie najlepiej tworzyć dane przez klasy w testach odpowiadające tym używanych w implementacji w połączeniu z generic builderem - https://howtocodetutorial.wor[...]ic-builder-pattern-in-java-8/

I możemy już dowolnie zbudować obiekt z nazwą wyszczególnioną w nazwie metody statycznej, przykład
TestObjects.anObjectWithInvalidId()

nohtyp

@danek Nie wiem czy to jest jakiś duży problem, ale mam wrażenie, że ludzie pisząc testy, zapominają o tym, że można wydzielać metody. O co mi chodzi? Nie zapominają, często jest tak, że refaktor testów leci na etapie utrzymania, bo do tego czasu jeszcze wiele rzeczy może się zmienić. Poza tym z doświadczeniem człowiek mniej czasu chce poświęcać na tworzenie złożonych testów.

danek

@nohtyp: ale nie chodzi mi o złożone testy, tylko o czytelne

nohtyp

Jeśli chcesz mieć czytelne to pisz dobry kod, w twoim przypadku klasa MatchInfo to problem, ponieważ ma rozbudowaną liczbę parametrów wejściowych. Czy jesteś pewien, że na tej liście czegoś nie zapomniałeś? Ogólnie słabo czyta się kod klas, które przyjmują nie wiadomo co i poprawa tej rzeczy nie tylko wpływa na czytelność testów, ale i kodu produkcyjnego. Zamiast klepać testy bezmyślnie, pomyśl czy rzeczywiście robienie opakowań dla takich wywołań ma sens.

danek

To jest json serializowany do DTOsa, normalne IDE podpowiada co jest czym i nie da się zapomnieć ;) Fakt, niefortunny przypadek, ale nie to jest główną myślą

nohtyp

W takim razie słabe to jest, a przykład jest spoko bo pokazuje, brak myślenia przy promowaniu kolorowych technik. https://stackoverflow.com/a/7962503

danek

Tylko jak rozbijesz to na kilka klas to nadal problem pozostanie ten sam. Chodzi mi o to, żeby w metodzie klasy unikać zbędnego tworzenia bardziej złożonych danych wejściowych, żeby nie zaburzały czytania. Co do samej klasy, to tak, nie jest idealna, ale lenistwo wygrało z ortodoksyjnością ;) Klasa jest tworzona wprost tylko gdzies w jakims smutnym mapperze, a potem unika się zbędnego getX().getY().getZ();

danek

Aczkolwiek, dobrym tematem do przemyślenia jest, na ile 'ulepszać' DTOsy, żeby tworzenie ich było 'ładniejsze'. Czy warto? Na ile warto?