Wstrzykiwanie zależności a testy jednostkowe - złoty środek

13

Brakuje na forum ostatnio ciekawych wątków, więc spróbuje ratować poziom! Zastanawiałem się ostatnio po wymianie zdań z @jarekr000000 gdzie jest złoty środek jeśli chodzi o wstrzykiwanie zależności, szczególnie takie zarządzane przez kontener IoC. Ciekawi mnie wasze zdanie na ten temat :)
Rozważmy hipotetyczny kod, który będzie obrazował dylemat:

public class MagicService {
    private DataCollector dataCollector;
    private DataExtractor dataExtractor;
    private DataTransformer dataTransformer;
    private DataSelector dataSelector;
    private ResultGenerator resultGenerator;
    private OutputFormatter outputFormatter;

    public Optional<Output> performComplexCalculations(Input input){
        try {
            RawData rawData = dataCollector.collectData(input);
            List<RelevantData> relevantData = dataExtractor.extractRelevant(rawData);
            List<AccessibleDataFormat> accessibleData = dataTransformer.transformToAccessibleFormat(relevantData);
            List<AccessibleDataFormat> filteredData = dataSelector.filter(accessibleData);
            List<GeneratedResult> generatedData = resultGenerator.generate(filteredData);
            return Optional.of(outputFormatter.formatOutput(generatedData));
        }catch(IOException ex){
            //handling1
        }catch(DataFormatException ex){
            //handling2
        }catch(GenerationException ex){
            //handling3
        }
        return Optional.empty();
    }
}

Na potrzeby dyskusji załóżmy że wszystkie te klasy będące zależnościami są mocno skomplikowane - możliwe że to są entry pointy do całych modułów z kupą kodu.
Załóżmy jednocześnie, ze to jest jedyne miejsce gdzie te klasy są użyte (czyli to nie są jakieś utilsy czy generyczne serwisy potrzebne w różnych miejscach)
Nasuwa się teraz pytanie: w jaki sposób dostarczyć te zależności do naszej klasy:

  1. Po prostu utworzyć sobie te obiekty za pomocą new, w końcu to są nasze wewnętrzne mechanizmy.
  2. Wstrzyknąć je z zewnątrz, czy to ręcznie za pomocą konstruktora/setterów czy za pomocą IoC.

Różnica wychodzi kiedy postanawiamy napisać testy jednostkowe dla naszej metody.

Jeśli utworzymy te zależności za pomocą new to w zasadzie nie ma możliwości przetestowania podanej metody w oderwaniu od działania jej zależności (pomijam akcje w stylu mockowanie wywołania new) - jedyne testy jakie możemy wykonać to takie quasi-integracyjne. Przygotowanie zestawy danych wejściowych może być tutaj mocno złożone. Dodatkowo pojawia się potencjalny problem z testowaniem wyjątków, bo nie każdy jest łatwo wywołać przy pracy z prawdziwymi obiektami. Co więcej jeśli pojawi się bug w którejś z naszych zależności do wywalą się zarówno jej testy jak i test dla naszej klasy, co potencjalnie utrudni poszukiwanie źródła problemu.
Na plus jest to, że napisanie samego testu jest względnie proste a test będzie dodatkowo trochę dokumentował kod, bo będziemy musieli przygotować w miarę realne zestawy danych wejściowych. No i będziemy mieli więcej testów wykonujących nasz kod.

Jeśli zamiast tego zależności zostaną wstrzyknięte to uzyskujemy możliwość testowania naszej metody w zupełnej izolacji - możemy mockować wszystkie zależności i sprawdzić sam przepływ naszej metody bez zastanawiania się czy jej zależności działają czy też nie. Możemy łatwo symulować wszystkie sytuacje (poprawne oraz wyjątkowe). Mamy wysoką izolacje więc bug w którejś z zależności wywali tylko test tej konkretnej zależności.
Możemy za to mieć bardzo skomplikowanym setup testu, kiedy trzeba ustalić bardzo dużo zachowań, w efekcie taki test jest mało czytelny.

Chętnie posłucham co o tym myślicie :)

5

Ale przecież ten kod nic nie robi poza integrowaniem ze sobą kilku komponentów i przekazywaniem danych między nimi. Nie widzę powodu aby w ogóle go unit-testować.

Jeśli chodzi o testy, to w najprostszej wersji mogłoby być tam nawet new. Napisałbym testy integracyjne działające na jakiś testowych danych, aby sprawdzić, czy komponenty się prawidłowo dogadują.

Oczywiście użycie new jest niewskazane z innych powodów - zapewne użytkownik lub admin tego systemu chciałby mieć jakiś wpływ na konfigurację każdego z komponentów (a może nawet wybór konkretnych imlementacji, których może być więcej). Można to osiągnąć na szybko frameworkiem DI. Można też tak jak my to robimy mieć jakiś własny plik konfiguracyjny, który jest zaczytywany przez odpowiedni moduł powołujący do życia i konfigurujący te pod-moduły a następnie wstrzykujący je do MagicService np. w konstruktorze. Wtedy testy integracyjne mogłyby mieć nawet swój zestaw takich plików konfiguracyjnych. I te pliki konfiguracyjne dla testów wymuszałyby pobieranie danych z innych źródeł niż później na systemie uruchomionym na produkcji.

0

No ok. To skoro ja zostałem wezwany to się wypowiem. Co prawda mistrzem testów nie jestem, nie mniej jednak piszę je. Na potrzeby tego wpisu przetestujemy prezenter (który jest wstrzykiwany z zależnościami, również podawane przez DI). I tak oto mamy

class MainPresenter(val service: Service,
                    val schedulerWrapper: SchedulerWrapper,
                    val context: Context,
                    val navigator: NavigatorApi,
                    val sharedPreference: SharedPreference) : BasePresenter<MainContracts.View>(), MainContracts.UserAction {

    override fun subscribeDownload(parameter: String) {
        schedulerWrapper.scheduleObservable(
                service.getSomeFromService(parameter),
                { items ->
                    setActions(items)
                    view.loadItemToView(items)
                },
                { e -> view.showError() },
                { view.hideProgress() })
    }

}

W takim prezenterze testuję interakcję z widokiem (robię verify na metodzie loadItemToView(list: List<Object>))

Zatem mój test wygląda tak:

  private MainPresenter presenter;
    private Service mockOfApi;
    private MainContracts.View mockOfView;

    @Before
    public void setUp() throws Exception {

        mockOfApi = mock(Service.class);
        mockOfView = spy(MainContracts.View.class);
        presenter = new MainPresenter(
                mockOfApi,
                spy(AndroidTestScheduler.class),
                mock(Context.class),
                mock(Navigator.class),
                mock(SharedPreference.class));
        presenter.setView(mockOfView);
 }

    @Test
    public void shouldLoadDataListToView() throws Exception {
        when(mockOfApi.getSomeFromService("parameter"))
                .thenReturn(just(givenData()));
        presenter.subscribeDownload("parameter");
        verify(mockOfView).loadItemToView(any());
    }

Omijam całkowcie DI mock'ując i spy'ując elementy, a przy okazji testuję metody. Może nie jest to dokońca o to o co Ci chodziło @Shalom, ale ja piszę w Android, więc testy, jak i samo DI trochę kuleje. A może jest to po prostu mój brak doświadczenia. Nie mniej jednak, testy spełniają swoją rolę.

0

Moim zdaniem ciężko argumentować niewłaściwość użycia DI. Nie podoba mi się jednak taka zależność modułu od zewnętrznych działań - chociażby od konieczności wstrzyknięcia zależności. Na takim module nie zrobimy copy-paste jednego pliku ale musimy martwić się o odpowiednią budowę obiektu. Niefajne. Odpowiedzialność za obiekt nagle jest w trzech klasach zamiast w jednej. Ponadto mam wrażenie że niektórzy - chyba chcąc być modni - na siłę wciskają DI wszędzie gdzie DI da się wcisnąć. Niedługo każdego inta będą do IoC ładować. Trzeba pamiętać, że nie wszystko należy testować (naprawdę!) a takie ładowanie DI na siłę gwałci KISS i tym podobne.
No i wspomniana już przez @Shalom wada DI - złożoność kodu. Żeby zmienić nasz MagicService na DI-friendly musimy dołożyć obrzydliwy, 6-argumentowy konstruktor, 6 interfejsów (które bardzo często mają tylko jedną implementację), pare fabryk i IoC. Też niefajne.

1

Na początek, zastanowiłbym się jakie jest zadanie tego serwisu i czy w ogóle jest sens go testować. Myślę że powinniśmy zacząć od:

public class MagicService {
    public Optional<Output> performComplexCalculations(Input input){
        return null;//TODO
    }
}

Mamy dane wejściowe które wsadzamy do metody i być może dostaniemy dane wyjściowe. Brakuje informacji dlaczego nie dostaniemy go zawsze i w jakich przypadkach może się tak zdarzyć. Ale ok, załóżmy że ktoś miał fantazję i jeżeli dane wejściowe będą błędne to nie otrzymamy nic. Czyli powiadamianie o błędach 1/0. Testy powinny więc zawierać błędne dane i sprawdzać czy jest empty() i poprawne, oraz sprawdzać czy wynik jest oczekiwany.

Przechodząc jednak do implementacji, okazuje się że wszystko co istotne dzieje się w zewnętrznych usługach, a ta jedynie je integruje i ew. wrzuca coś do logów. Nie możemy wiec stworzyć poprawnego/niepoprawnego zestawu danych, jeżeli to inne serwisy decydują co jest dobre a co złe. Tutaj więc faktycznie pozostają testy integracyjne.

Załóżmy jednak, że metoda zwraca sam Output (bez Optional) i jest to jedna z implementacji CorrectOutput/IncorrectOutput. Np. w Input jest podany specyficzny algorytm, na podstawie którego wybierany jest serwis do wypełniania. Jeżeli podany algorytm jest niezaimplementowany, to wyrzuca IncorrectOutput z konkretnym komunikatem. To możemy przetestować. Jeżeli któraś z usług wyrzuci wyjątek to też, leci IncorrectOutput, ale z wcześniej ustalonym mapowaniem błędu. Nie możemy przewidzieć co dostaniemy, ale komunikat i tak powinniśmy zmapować.

Czyli np, interfejs DataTransformer wyglądałby tak:

public interface DataTransformer {
    Algorithm transformerAlgorithm();
    ... transformToAccessibleFormat(...) ...
}

Do usługi wstrzykujemy kolekcję DataTransformerów i na jej podstawie budujemy mapę obsługiwanych algorytmów.

W tym wypadku robimy testy, a usługi musimy zamockować:

  • Dane są poprawne, tj lista DataTransformerów zawiera algorytm z Input i żadna z usług nie wyrzuciła wyjątków
  • Dane są niepoprawne, lista DataTransformerów nie zawiera wymaganego algorytmu i testujemy czy błąd jest taki jak oczekiwaliśmy.
  • Dane są niepoprawne, tj Jedna z usług wyrzuciła wyjątek z komunikatem, a my testujemy czy nasz IncorrectOutput zawiera odpowiednio sforamtowaną wiadomość.

I w tym wypadku mamy 3 opcje implementacji.

  • Tworzenie usług przez new. Tragedia przy testowaniu. Zabawa z ustawianiem mocków wymaga znajomości implementacji. Najgorsza opcja.
  • Ustawianie/wstrzykiwanie przez settery lub wstrzykiwanie przez pola. Łatwiej testować, chociaż do wstrzykiwania przez pola musielibyśmy mieć odpowiednie narzędzie lub ręcznie bawić się z refleksją. Settery natomiast sugerują że implementacje serwisów mogą zmienić się w czasie. A w praktyce jak często się tak dzieje?
  • Ustawianie/wstrzykiwanie przez konstruktor. Najłatwiejsze testowanie i po stworzeniu, usługa jest spójna i niezmienna (o ile zależności wewnątrz też są niezmienne ;)). Moim zdaniem to jest właśnie złoty środek.

Nie widzę minusów wstrzykiwania przez konstruktor i plusów tworzenia obiektów przez new wewnątrz obiektu. Jeżeli konstuktor miałby kilka parametrów które i tak w większości byłyby takie same, to dla ułatwienia można przecież stworzyć metodę statyczną default(), która zwróci nam to, co bezparametrowy konstuktor tworzący zależności przez new. Obok mielibyśmy publiczny konstruktor dla innych usług/testów.

IoC to taki as w rękawie Javy i zgodzę się że jest to ogromna zaleta. Problem w tym że wielu ludzi słysząc IoC, myśli o kontenerze do wstrzykiwania zależności, a przecież to jedynie dobra technika odwrócenia zależności, niezależna od technologii. Java ma po prostu sporo dobrego softu wspomagającego.

1

W widzę że CERN pamięta o biednych niewykształconych :) tylko widzisz @Shalom musiałem dla Ciebie nowe konto zrobić mam nadzieje że mi wybaczysz że nie piszę z @zsuiyram9

Dobra, ale wracając do tematu.

  1. Po prostu utworzyć sobie te obiekty za pomocą new, w końcu to są nasze wewnętrzne mechanizmy.

to odpada :) z powodów które wymieniłeś :D

Możemy za to mieć bardzo skomplikowanym setup testu, kiedy trzeba ustalić bardzo dużo zachowań, w efekcie taki test jest mało czytelny.

Mamy z tym bardzo duży problem, szczególnie w przypadkach kiedy każdy taki serwis, czyli u Ciebie np:

    private DataCollector dataCollector;
    private DataExtractor dataExtractor;
    private DataTransformer dataTransformer;
    private DataSelector dataSelector;
    private ResultGenerator resultGenerator;
    private OutputFormatter outputFormatter;

pod spodem robi REST calla gdzieś indziej :/ każdy taki serwis może zakończyć się średnio 4 responsami (np 200, 404, 401 albo pińćset) ( każdy taki serwis trzeba zamockować, mockowanie rest clienta jest biedne, także kończysz potem z milionem przypadków jsonów przekazywanych do np: wiremocka :/ ) - z matematyki dobry nie byłem ale można se policzyć że jakbyśmy chcieli obłużyć w testach każdy przypadek to ... generalnie nie jest fajnie.

U nas to wygląda tak, że także z tych property jest wstrzykiwane (generalnie mamy DI - ale czasem zastanawiamy się czy na pewno jest to potrzebne) - ale równie dobrze, tak jak napisałeś przez zwykłe settery bez DI (znaczy to nadal będzie DI, ale bez kontenera :/ ).

także ten Twój kod

public class MagicService {
	@Autołajerd
	private DataCollector dataCollector;
	@Autołajerd
	private DataExtractor dataExtractor;
	@Autołajerd
	private DataTransformer dataTransformer;
	@Autołajerd
	private DataSelector dataSelector;
	@Autołajerd
	private ResultGenerator resultGenerator;
	@Autołajerd
	private OutputFormatter outputFormatter;

	public Optional<Output> performComplexCalculations(Input input){
        try {
            RawData rawData = dataCollector.collectData(input);
            List<RelevantData> relevantData = dataExtractor.extractRelevant(rawData);
            List<AccessibleDataFormat> accessibleData = dataTransformer.transformToAccessibleFormat(relevantData);
            List<AccessibleDataFormat> filteredData = dataSelector.filter(accessibleData);
            List<GeneratedResult> generatedData = resultGenerator.generate(filteredData);
            return Optional.of(outputFormatter.formatOutput(generatedData));
        }catch(IOException ex){
            //handling1
        }catch(DataFormatException ex){
            //handling2
        }catch(GenerationException ex){
            //handling3
        }
        return Optional.empty();
	}
}

skończyłbym z duzą kupą testów unitowych gdzie wszyskie serwisy są pomockowane a ja mogę sobie sprawdzić ja MagicSerwis ogarnie oczekiwane wyjątki i dostane oczekiwanego Optionala.empty

czyli duzo takich :

@Test
public void shouldReturnOptionalEmptyWhenOutputFormatterThrowsGenerationException()
{
 //given
 when(outputFormatter.formatOutput).throws(DataFormatException)
 Input input = ...
 
 //when
 Optional<Output> result = magicSerwis.outputFormatter.formatOutput
 //then
 
 chceMiec(result.empty)
}

  • Czyli magicSewis to żywy obiekt, wszystko inne mockito

I kilka integracyjnych z setupem danych - ale tylko hepi paf - nic więcej, barak jakiś fancy corner kejsów

P.S nie wiem czy o to dokładnie Ci chodziło

1

Ta klasa zbiera do kupy wszystkie moduły, więc testowanie tego jednostkowo wydaje mi się średnio użyteczne — zamockujesz wszystko, podłożysz jakieś dane i co właściwie sprawdzisz? Że metody zostały wywołane w dobrej kolejności? Niby można, tylko po co, przecież teoretycznie każdy z tych komponentów powinien być przetestowany osobno, tak samo logika obsługująca wyjątki powinna mieć swoje testy, więc dla tej klasy wystarczy test integracyjny. Test jednostkowy mógłby się przydać wtedy, gdybyśmy chcieli testować negatywne scenariusze, a ich wywołanie z prawdziwymi implementacjami byłoby upierdliwe (przez jakieś timeouty itp).

Teraz odnośnie wstrzykiwania: zasadniczo @Krolik ma rację i cała konfiguracja powinna być złożona do kupy czymś osobnym, może to być kontener DI, może to być własna refleksja. Jeżeli nie przewidujemy potrzeby rekonfigurowania aplikacji w jakiś ludzki sposób (albo po prostu mamy jedną implementację dla każdej zależności), to jak dla mnie new wystarczy, o ile jesteśmy w stanie wszystko przetestować integracyjnie. Jeżeli jednak musimy mockować aby przetestować bardziej wydumane scenariusze, to zwykłe DI wystarczy.

Inną opcją jest zrobienie fabryki tworzącej ten serwis. Wtedy w testach mamy fabrykę podstawiającą mocki, a w rzeczywistej aplikacji mamy fabrykę tworzącą wszystko przez new/refleksję, przy czym nie różni się to znacząco od kontenera DI, ale może być czytelniejsze.

0

A właściwie to co jest złego w użyciu DI i wstrzykiwaniu przez konstruktor (ewentualnie przez pole?

3

Mnie nie przeraża konstruktor mający i dwadzieścia argumentów. Po to jest kontener IoC, żeby się tym nie przejmować, zwłaszcza że alternatywą przy testach - dla zachowania ich prostoty - są tylko "słabe" (loose, weak) częściowe (partial) mocki i zmiana private na internal.
Dużo małych serwisów (SRP) powiązanych ze sobą przez DI (LSP, ISP, DIP) na możliwie prostych i uniwersalnych zasadach (coś w stylu Class : IClass, żeby było w miarę KISS) umożliwia pisanie testów krótkich, a zatem przejrzystych i łatwych do zrozumienia. Gdy zachodzi potrzeba zmiany sposobu działania przykrytej testami metody, to zmiany w testach dotyczą kilku krótkich metod.
Dla kontrastu wyobraźmy sobie odwrotną sytuację. Dysponujemy dokładnie takim kodem, jak podany przez Shaloma. Przykrycie testami takiego kodu oznacza, że robię test integracyjny. Czarna skrzynka, wsadzam określone dane, oczekuję określonego zachowania. Zatem muszę przygotować wszystkie dane dla wszystkich serwisów wykorzystywanych przez testowaną metodę, oraz wszystkie dane dla wszystkich serwisów używanych w środku przez serwisy itp. Na koniec mamy fajny test, bo mamy w dupie jak zaimplementowana w środku jest testowana metoda, implementacja może się zmienić, a test będzie cały czas ok... Ale zaraz zaraz:

  • zmiana implementacji może oznaczać zmianę używanych danych, więc trzeba będzie zrefactorować długi, potencjalnie słabo zrozumiały test;
  • test jest długi, bo musieliśmy zasetupować sporo danych (oczywiście nie zawsze się tak dzieje i tu jest miejsce na znalezienie złotego środka - metody wymagające łatwego setupowania bardzo małej ilości danych można testować integracyjnie);
  • jak zasetupować dane zaciągane klientami z webserwisów z innych aplikacji webowych? Bez mocka ani rusz! Tu taka anegdotka - kiedyś co jakiś czas na środowisku CI co jakiś czas (!) wywalał nam się jeden test, u nas lokalnie działał, na CI czasem nie. W końcu wzięliśmy to pod lupę i okazało się, że ktoś nie zamockował klienta webserwisu, używanego przez testowaną metodę; webserwis (testowa wersja innej aplikacji) zwykle działał, ale czasem apka była wyłączana i wtedy u nas na CI leciały błędy...
  • jak przetestować obsługę wyjątków i nietypowych sytuacji? ponownie, bez mocków ani rusz;
  • testy integracyjne są drogie, tzn. wymagają czasu (wykonują się znacznie dłużej od testów na mockach) i zasobów (np. bazy danych).
    Oczywiście testy integracyjne muszą powstać, bo w pewnym momencie odbijamy się np. od bazy danych. Z mojego doświadczenia z ORM EF wynika, że o ile od wersji bodajże 6 mockowanie EF jest w końcu możliwe, o tyle jest bardzo czasochłonne. Prościej i szybciej jest zrobić sobie warstwę abstrakcji do setupowania obiektów na bazie (Ensure - nie wiem, czy to jakiś wzorzec projektowy jest) i testować się z bazą.

Moim zdaniem priorytetem powinno być TTB (time-to-business) oraz jakość kodu (bo do małych aplikacji, gdzie zamiast "jakość" jest "jakoś", nie pisze się testów). Jeśli tylko jest czas na dobre napisanie kodu (złe zawsze się mści w długo żyjących aplikacjach), to należy go napisać w miarę zgodnie z SRP oraz wstrzykiwać wszystkie zależności. Dzięki temu kod będzie czytelniejszy, a testy krótsze i szybsze.

0

Ciekawi mnie jak nisko schodzić z DI. Jeśli np. mój serwis potrzebuje Introspektora do analizy obiektów (z życia wzięte: walidator który sprawdza między innymi czy którekolwiek pole obiektu jest ustawione na nie-nulla, potencjalmych klas do walidacji dużo więc łatwiej zrobić jeden walidator który refleksją to zrobi). Czy taki Introspektor powinien być wstrzykiwanym serwisem czy powinien być stworzony przez new w naszym walidatorze?

1

Zgadzam się z @Afish i @generalnieNieLubieAurel

Tutaj nie ma i być nie może mowy o sensownym testowaniu tego, bo to przecież tylko fasada.

Widzę tutaj możliwość napisania tylko jednego testu - BDD - który sprawdzi czy:

  1. dataCollector.collectData zostanie wywołane z podanym parametrem input
  2. dataExtractor.extractRelevant zostanie wywołane z tym, co zwróciło dataCollector.collectData
    itp.

Całość testu ogarnie annotacja w stylu @Spy z Mockito - i tyle.

Dla hardcore'ów można jeszcze przetestować exception handling, tzn. sprawdzić, czy handlery nam prawidłowo działają. Za bardzo nie znam ich założeń, więc nie pomogę.

0

To zależy od tego, jak nisko chcesz schodzić z testami. Jeśli traktujesz walidator jako zło konieczne, to olewasz go w testach. Jeśli chcesz zagwarantować jego używanie w kodzie i poprawne zachowanie aplikacji, to uwzględniasz go w testach. Jednak nadal nie musisz robić mocka, a więc nie musisz wstrzykiwać walidatora, bo możesz użycie walidatora potraktować testami integracyjnymi. Jednak jeśli potrzebujesz sprawdzić, że metoda walidatora została poprawnie wywołana z poprawnymi parametrami, to osiągniesz to tylko na mocku.
W tym konkretnym przypadku moim zdaniem walidator powinien być sam przykryty testami, a potem jego wywołania zamockowane. To drugie po to, żeby mieć pewność, że w testowanej metodzie walidator został wywołany. W przeciwnym wypadku będziesz musiał dopisywać do każdej metody używającej walidatora dodatkowe testy na case, kiedy masz nieprawidłowe dane i walidator aktywnie włącza się do procesu przetwarzania danych.

0

IMO albo powinniśmy korzystać z DI, albo nie. Nie powinniśmy robić rzeczy w style, że część logiki/serwisów czeka na wstrzyknięcie zależności, a część sama sobie je ustawia przy pomocy new, ponieważ w dużych projektach może dojść do sytuacji gdzie kolega się na nas pogniewa, kiedy podmieni implementację w DI a rezultatu ni jak nie zobaczy.

Osobiście mi koncepcja IoC się podoba i stosuję ją za każdym razem (z opisanych powyżej przyczyn). Zdarzyło mi się jednak korzystać z takich konstrukcji (do wymuszenia domyślnej implementacji per service):

public class IntegrationService
{
     public IntegrationService(ILogger logger = null)
     {
          this.logger = logger ?? new EventViewerLogger();
     }
}
0

To taki follow up question jeszcze: czy nie uważacie że takie opieranie aplikacji na wstrzykiwanych bezstanowych serwisach to nie jest trochę takie programowanie strukturalne, podobne do tego jakbyście wszystko opierali o statici? I jak się to ma też do postulatów z zakresu DDD i lokowania logiki biznesowej w obiektach domenowych?

1
Shalom napisał(a):

To taki follow up question jeszcze: czy nie uważacie że takie opieranie aplikacji na wstrzykiwanych bezstanowych serwisach to nie jest trochę takie programowanie strukturalne, podobne do tego jakbyście wszystko opierali o statici?

No ba, nie na darmo domyślnym scope'm w Springu jest przecież Singleton :)

EDIT: tak mnie naszło - zauważ, że obecnie programowanie ewoluuje w stronę prostoty, a nawet prostactwa. Z powodu nadmiaru zasobów aplikacje dzieli się na malutkie, niezależne bloki, które następnie się spina w większe całości. Z jednej strony daje to czytelność takiego kodu - ale z drugiej strony jest to skrajnie nieefektywne (chociażby i referencje w Javie muszą być przecież kopiowane N razy przy przekazywaniu przez parametry). Sytuacja zmienić się raczej nie zmieni, gdyż złożoność aplikacji rośnie wolniej niż wydajność komputerów.

8

Od razu zaznaczę, że koduję w Scali, więc tutaj podejście może być trochę inne, ale do rzeczy:

Moim zdaniem mockowanie i stosowanie kontenerów DI to przerost formy nad treścią. Oba utrudniają nawigację po kodzie oraz wręcz są moim zdaniem błędogenne. Ja preferuję lekkie testy integracyjne i wstrzykiwanie bezpośrednio przez konstruktor.

Wstrzykiwanie przez konstruktor w Scali ma niewielki narzut w kodzie. Zamiast:

class Service {
  val dependency1 = new Dependency1()
  ...
  val dependencyN = new DependencyN()

  def action() = ???
}

Mam:

class Service(dependency1: Dependency1, ..., dependencyN: DependencyN) {
  def action() = ???
}

object Service extends Service(new Dependency1(), ..., new DependencyN())

Zamiast produkcyjnego singletona (object) obok każdej klasy można stworzyć produkcyjny zestaw zależności w osobnym singletonie, czyli:

object ProductionSetup {
  val dependency1 = new Dependency1()
  ...
  val dependencyN = new DependencyN()
  val service = new Service(dependency1, ..., dependencyN)
}

Wszystko widać jak na dłoni i nawigacja po kodzie jest bajecznie prosta i szybka, niewymagająca żadnych specjalnych wtyczek do IDE.

Nie ma żadnej refleksji i frameworków DI. Dodatkowego kodu wcale nie ma dużo. Enkapsulacja jest zachowana - parametry z głównego konstruktora są domyślnie prywatne. Tworzenie nowej instancji obiektu z innymi zależnościami jest trywialne.

Teraz opiszę problem z mockowaniem. Załóżmy, że mamy DSLa i:

object.method(params) returns result

oznacza mniej więcej to samo co:

when(object.method(params).thenReturn(result)

(niewielki wzrost czytelności)
Przykładowe mockowanie to:

val mockA1 = mock[A]
val mockA2 = mock[A]
val mockB1 = mock[B]
mockB1.a returns mockA1
val mockB2 = mock[B]
mockB2.a returns mockA2

Problem polega na tym, że łatwo się rąbnąć z numerkami. Nie ma tutaj jedynej słusznej kolejności uszeregowania wywołań. Wraz z rozrostem grafu decyzja co do uszeregowania będzie coraz trudniejsza, a samo rozpoznanie użytego szeregowania w danym miejscu też trudniejsze. Gdy zamiast tego zrobimy test integracyjny to graf zależności staje się dużo bardziej oczywisty, a konwencje szeregowania niespecjalnie potrzebne. Np (nulli w Scali się nie stosuje i nie sprawdza na ich obecność - zamiast tego jest Option; tutaj null jest po to by zaznaczyć, że dany parametr nie powinien być jakkolwiek użyty w teście - jego użycie powinno spowodować NPE):

val fakeC1 = C(null, "something1", null)
val fakeC2 = C(null, "something2", null)
val fakeB1 = B(fakeC1, realD)
val fakeB2 = B(fakeC2, realD)

Dla mnie jest to znacznie czytelniejsze. Zamiast wielu linijek mockX returns something mam jedną linijkę z tworzeniem X'a z kilkoma parametrami do konstruktora.

Przytoczono tutaj wadę, że stosowanie testów integracyjnych zamiast mockowania sprawia, że przy popsuciu jakiejś klasy posypie się więcej testów "niż powinno". Sprawa jest taka, że powinien posypać się przede wszystkim test klasy, którą pozmienialiśmy i tam powinniśmy szukać wskazówek do rozwiązania problemu. Co jeśli wysypią się testy dla innych klas niż ta którą zmodyfikowaliśmy? No, to pewnie oznacza, że nasz test dla tej klasy był niewystarczający i przypadkowo (rzutem na taśmę) problem został wykryty gdzie indziej. Wypada się tylko cieszyć - gdyby nie to to nie wykrylibyśmy problemu w ogóle.

Był argument o dwudziestu parametrach w konstruktorze - jeśli tak jest to trzeba je opakować w klasę. To podstawowa zasada czystego kodu. Może to jednak sugerować, że klasa ma zbyt wiele zależności, więc możliwe, że zbyt dużo rzeczy robi naraz i przydałaby się reorganizacja kodu.

Był argument o wydajności testów integracyjnych, a więc bajka o dostępie do bazy danych. Tu sprawa jest prosta - można sklecić testową infrastrukturę imitującą dostęp do bazy danych. Podobnie można zrobić imitacje zewnętrznych serwisów. Z czymś takim testy są wystarczająco szybkie.

Był argument o testowaniu wyjątków. W Scali to nie problem, bo ogólna konwencja jest taka, że wyjątki łapiemy najwyżej jak się da i opakowujemy w Either/ Try/ Option/ cokolwiek mające sens.

Mocki mają tę wadę, że utrudniają refaktoryzację. Zmiana sygnatury metody wymusza zmianę mockowania w wielu miejscach (w testach dla wielu klas), podczas gdy przy stosowaniu testów integracyjnych zmiana jest tylko w teście dla klasy w której jest ta metoda.

Mam podejście pragmatyczne i jeśli coś jest wystarczające, a eleganckie to tego używam i nie kieruję się w stronę rozwiązań modnych/ super elastycznych/ cokolwiek, które w danym miejscu nijak nie są potrzebne. YAGNI panowie. Dla mnie priorytetem jest czytelność kodu, a na to wpływa między jego zwięzłość i szybkość nawigowania (oprócz tak oczywistych rzeczy jak sensowne uporządkowanie). Zwięzłość kodu produkcyjnego ma u mnie priorytet nad zwięzłością kodu testującego, bo wielokrotnie częściej czytam kod produkcyjny.

Na koniec muszę dodać, że nasza architektura jest oparta na mikroserwisach, a więc testowanie może wyglądać inaczej niż w kobyłach. Mikroserwis ma to do siebie, że zwykle ma niewielkie warstwy dostępu do I/O (baza, http, kolejki, etc), a co za tym idzie łatwo jest stworzyć imitacje tych warstw i je używać w testach.

EDIT: tak mnie naszło - zauważ, że obecnie programowanie ewoluuje w stronę prostoty, a nawet prostactwa. Z powodu nadmiaru zasobów aplikacje dzieli się na malutkie, niezależne bloki, które następnie się spina w większe całości. Z jednej strony daje to czytelność takiego kodu - ale z drugiej strony jest to skrajnie nieefektywne (chociażby i referencje w Javie muszą być przecież kopiowane N razy przy przekazywaniu przez parametry). Sytuacja zmienić się raczej nie zmieni, gdyż złożoność aplikacji rośnie wolniej niż wydajność komputerów.

Łojej. Przekazywanie referencji jest takie zasobożerne? Sprawdzałeś to?

Dzielenie aplikacji na niezależne bloki i ich łatwe składania to święty Graal informatyki. Możliwość składania aplikacji jak klocków lego dałaby duże możliwości, bo w stanie obecnym integrowanie się z kolejnym klockiem (np aplikacją z zewnątrz) to straszny i długotrwały ból.

1
Shalom napisał(a):

To taki follow up question jeszcze: czy nie uważacie że takie opieranie aplikacji na wstrzykiwanych bezstanowych serwisach to nie jest trochę takie programowanie strukturalne, podobne do tego jakbyście wszystko opierali o statici? I jak się to ma też do postulatów z zakresu DDD i lokowania logiki biznesowej w obiektach domenowych?

Zazwyczaj aplikacje LOB nie potrzebują nie wiadomo jak rozbudowanej domeny, bo są zwykłymi CRUD-ami. Anemiczny model dziedziny rzeczywiście wygląda jak pisanie na staticach (pewnie nawet IDE będzie podpowiadało, że połowę metod w serwisach można pozmieniać na statyczne), ale nie uważam tego za problem, o ile tylko dla mojej aplikacji jest to wystarczające.

Wibowit napisał(a):

Był argument o wydajności testów integracyjnych, a więc bajka o dostępie do bazy danych. Tu sprawa jest prosta - można sklecić testową infrastrukturę imitującą dostęp do bazy danych. Podobnie można zrobić imitacje zewnętrznych serwisów. Z czymś takim testy są wystarczająco szybkie.

Nie zgodzę się — właśnie o to chodzi w teście integracyjnym, żeby zintegrować komponenty i sprawdzić, czy one naprawdę razem działają. Już nie raz moja aplikacja przechodziła wszystkie testy, a potem okazywało się, że jednak ORM nie potrafi przetłumaczyć jakiegoś zapytania, mimo że z bazą in memory poradził sobie bez problemu.

1

Taki ladny flame sie zaczal, a ja odsypialem #HackZurich....

Kod dobrze pokazuje dlaczego wstrzykiwanie to bagno (patrz dyskusja o bezsensie testowaniu tego).

Ale da sie naprawic - @Shalom potrzebne do tego bedzie:

  1. Lista implementacji tych wstrzykiwanych klas (jak sie nazywaja).
  • Nazwy i rzucane wyjatki (inaczej zakladam pesymistycznie i potrzeba mi 18 testow...)
  1. Co robia te handling? bo wynikiem jest Optional - daja po prostu Empty?

}catch(IOException ex){
//handling1
}catch(DataFormatException ex){
//handling2
}catch(GenerationException ex){
//handling3

0

To jest w ogóle ciekawy przypadek kodu. O tym za chwilkę.

Kontener DI jest rozwiązaniem zdroworozsądkowym, ale prawdą jest "quest poboczny" - bezstanowe serwisy mogą być równie dobrze wypełnione metodami statycznymi i nie będzie różnicy pomiędzy takim kodem, a kodem w Pascalu (no dobra, w Pascalu już nie będzie można pisać na maturze). W efekcie otrzymamy potworka, z którym musimy rozwiązywać problemy jak powyższy.

Użycie DI będzie miało tę małą zaletę, że pozwoli nam na napisanie iluśtam mocków i symulowanie różnych ścieżek w kodzie. Spowoduje to oczywiście skomplikowanie testów, ale tu dochodzimy do ciekawego przypadku.

Jeżeli spojrzymy na ten kod jako na sekwencję, kolejnych funkcji takich, że

F(x)→y 
G(y)→z
H(z)→Option[?]

Czyli wynik jednej funkcji jest zgodny z typem argumentu kolejnej, to można cały ten kod wyrzucić do śmieci i zamiast niego napisać jakiś mechanizm generycznie składający funkcje. Wtedy funkcja DI ograniczy się do odpowiedniego uszeregowania istniejących komponentów. Warunkiem jest wiedza o typie wejścia i typie wyjścia poszczególnych elementów. W efekcie mamy graf gdzie wierzchołki reprezentują poszczególne typy, a krawędzie dopuszczalne przejścia (istniejące w jakimś repozytorium funkcje). Rola DI to wyszukanie ścieżki pomiędzy typem wejściowym i wyjściowym z uwzględnieniem punktów pośrednich wskazanych przez użytkownika.

Tyle tylko, że tego w Javie nie za bardzo idzie napisać w prosty sposób.

0
generalnieNieLubieAurel napisał(a):

Mamy z tym bardzo duży problem, szczególnie w przypadkach kiedy każdy taki serwis, czyli u Ciebie np:

    private DataCollector dataCollector;
    private DataExtractor dataExtractor;
    private DataTransformer dataTransformer;
    private DataSelector dataSelector;
    private ResultGenerator resultGenerator;
    private OutputFormatter outputFormatter;

pod spodem robi REST calla gdzieś indziej :/ każdy taki serwis może zakończyć się średnio 4 responsami (np 200, 404, 401 albo pińćset) ( każdy taki serwis trzeba zamockować, mockowanie rest clienta jest biedne, także kończysz potem z milionem przypadków jsonów przekazywanych do np: wiremocka :/ ) - z matematyki dobry nie byłem ale można se policzyć że jakbyśmy chcieli obłużyć w testach każdy przypadek to ... generalnie nie jest fajnie.

To jest nie fajne - ale robi sie znosne jak error handling zrzucisz na RxJava. Ale to niezalezna bajka.
Jak nie mam retry, last, albo jakis rownoleglych wywolan - to jade klasycznie tylko Either z jakims Status enumem i da sie to ogarnac. (Znowu zalozenie Either jest juz przetestowany) - czyli zasadniczo koncentruje sie na Happy Path.

0
jarekr000000 napisał(a):
generalnieNieLubieAurel napisał(a):

Mamy z tym bardzo duży problem, szczególnie w przypadkach kiedy każdy taki serwis, czyli u Ciebie np:

    private DataCollector dataCollector;
    private DataExtractor dataExtractor;
    private DataTransformer dataTransformer;
    private DataSelector dataSelector;
    private ResultGenerator resultGenerator;
    private OutputFormatter outputFormatter;

pod spodem robi REST calla gdzieś indziej :/ każdy taki serwis może zakończyć się średnio 4 responsami (np 200, 404, 401 albo pińćset) ( każdy taki serwis trzeba zamockować, mockowanie rest clienta jest biedne, także kończysz potem z milionem przypadków jsonów przekazywanych do np: wiremocka :/ ) - z matematyki dobry nie byłem ale można se policzyć że jakbyśmy chcieli obłużyć w testach każdy przypadek to ... generalnie nie jest fajnie.

To jest nie fajne - ale robi sie znosne jak error handling zrzucisz na RxJava. Ale to niezalezna bajka.
Jak nie mam retry, last, albo jakis rownoleglych wywolan - to jade klasycznie tylko Either z jakims Status enumem i da sie to ogarnac. (Znowu zalozenie Either jest juz przetestowany) - czyli zasadniczo koncentruje sie na Happy Path.

Nie jaruś, sorry ale im mniej rxjavy dla mnie lepiej. Zresztą... rxjava to nie jest technologi która opędza retraje na połączeniach do zewnętrznych serwisów - tutaj krytyczny jest np: timeout, ja chce mieć tam inną thread poole dla requestów a inną dla zwykłego przetwarzania (tak moge to opędzić rxjavą i jej schedulerami) etc. Od takich rzeczy jest https://github.com/Netflix/Hystrix - który jak będzie trzeba otworzy circuit breaker i zamknie. Batchowanie requestów to też nie jest coś co się zrobi w rxjavie bo nie do tego służy ta technologia .

0

No więc wywołany do tablicy przez @Shalom - postanowiłem zrobić z tego konkret.
Po pierwsze, można już chyba wątek przenieść do flame (i tak wiadomo było, że tam skończy).
Po drugie na Githubie są sklecone na szybko dwa odcinki (na razie jest daleko do rozwiązania, i wcale nie wiem czy skończy się tak jak myślę).
Odcinek to kod w branchu plus opowieść.

Odcinek 1 Początek https://github.com/javaFunAgain/magic_service_story/tree/10_BEGIN
Odcinek 2 Robimy to kompilowalne https://github.com/javaFunAgain/magic_service_story/tree/20_MAKE_IT_COMPILING

Oczywiście pull requesty i forki! są mile widziane.

ciąg dalszy może nastąpi....

0

I jeszcze odcinek 3: Pierwszy test (w Junit 5)
https://github.com/javaFunAgain/magic_service_story/tree/30_FIRST_TEST

11

O, @Shalom wreszcie zadał ciekawe pytanie, a nie znowu o formatki w swingu.

Shalom napisał(a):

Różnica wychodzi kiedy postanawiamy napisać testy jednostkowe dla naszej metody.

Panie, ten kod to prosty kontroler, który odpala odpowiednie metody faktycznych serwisów w odpowiedniej kolejności. Kto testuje jednostkowo kontrolery? Chyba tylko seniorzy z dwuletnim doświadczeniem.

Tu nie ma co testować jednostkowo, bo tu nie ma logiki. A skoro to jest coś na szczycie aplikacji, to i tak trzeba to przetestować integracyjnie. Testy jednostkowe niczego tu nie wniosą, właściwie przetestuje się tylko działanie frameworka mockującego.

Dodatkowo pojawia się potencjalny problem z testowaniem wyjątków, bo nie każdy jest łatwo wywołać przy pracy z prawdziwymi obiektami.

Jeśli chcemy przetestować realne sytuacje i prawdziwe wyjątki, to i tak nie zrobimy tego jednostkowo.

Co więcej jeśli pojawi się bug w którejś z naszych zależności do wywalą się zarówno jej testy jak i test dla naszej klasy, co potencjalnie utrudni poszukiwanie źródła problemu.

Dlatego najpierw odpalamy unit testy naszych składowych klocków, a jak one przejdą, to testy integracyjne całości.

Możemy za to mieć bardzo skomplikowanym setup testu, kiedy trzeba ustalić bardzo dużo zachowań, w efekcie taki test jest mało czytelny.

Główny problem z przygotowywaniem testów jest to, że ludzie tracą czas na robienie tego ręcznie zamiast użyć automockującego frameworka.

pingwindyktator napisał(a):

No i wspomniana już przez @Shalom wada DI - złożoność kodu. Żeby zmienić nasz MagicService na DI-friendly musimy dołożyć obrzydliwy, 6-argumentowy konstruktor, 6 interfejsów (które bardzo często mają tylko jedną implementację), pare fabryk i IoC. Też niefajne.

Sensowne kontenery IoC nie wymagają interfejsów do działania. Zresztą, nie spotkałem jeszcze nawet bezsensownego kontenera IoC, który by tego wymagał. :P
Sensowny kontener IoC nie wymaga też żadnych zmian w konfiguracji po zmianie sygnatury konstruktora.

Shalom napisał(a):

Czy taki Introspektor powinien być wstrzykiwanym serwisem czy powinien być stworzony przez new w naszym walidatorze?

To zależy od tego, czy jest zależnością czy też własnością albo narzędziem.

Shalom napisał(a):

To taki follow up question jeszcze: czy nie uważacie że takie opieranie aplikacji na wstrzykiwanych bezstanowych serwisach to nie jest trochę takie programowanie strukturalne, podobne do tego jakbyście wszystko opierali o statici? I jak się to ma też do postulatów z zakresu DDD i lokowania logiki biznesowej w obiektach domenowych?

Moim zdaniem DDD z tymi swoimi encjami, które operują same na sobie jest przereklamowane. Ale to bez znaczenia, bo z DDD jest jak z Agile - nikt go w praktyce nie używa, mimo że wszyscy o tym gadają.

Dobry kod jest bezstanowy, po prostu przetwarza wejście w wyjście. Jeśli nie ma stanu, to jest statyczny (nawet bez magicznego słowa kluczowego). Ale to nie znaczy, że jest strukturalny, jest raczej funkcyjny. Niestety w praktyce nie wszystko może takie być, bo poza przetwarzaniem danych musimy jeszcze obsługiwać efekty uboczne.

Wibowit napisał(a):

Był argument o dwudziestu parametrach w konstruktorze - jeśli tak jest to trzeba je opakować w klasę. To podstawowa zasada czystego kodu.

Jeśli wsadzisz te 20 parametrów do jednej klasy, to będziesz musiał jej dodać konstruktor z 20 parametrami, więc wiele to nie zmienia.

W przypadku jakichś kontrolerów (jak w pierwszym poście), te 20 parametrów to nie jest tragedia, bo to normalne, że coś, co składa ileś wywołań i je wykonuje, musi najpierw dostać te wywołania.
W przypadku serwisu to faktycznie może świadczyć o złamaniu SRP. Ale technicznie to i tak nie problem, bo to rozwiązuje kontener IoC.

Był argument o wydajności testów integracyjnych, a więc bajka o dostępie do bazy danych. Tu sprawa jest prosta - można sklecić testową infrastrukturę imitującą dostęp do bazy danych. Podobnie można zrobić imitacje zewnętrznych serwisów. Z czymś takim testy są wystarczająco szybkie.

Jak dla mnie, to imitacja i mock to to samo.

Mocki mają tę wadę, że utrudniają refaktoryzację. Zmiana sygnatury metody wymusza zmianę mockowania w wielu miejscach (w testach dla wielu klas), podczas gdy przy stosowaniu testów integracyjnych zmiana jest tylko w teście dla klasy w której jest ta metoda.

Tylko jeśli mockujesz ręcznie.

Na koniec muszę dodać, że nasza architektura jest oparta na mikroserwisach, a więc testowanie może wyglądać inaczej niż w kobyłach. Mikroserwis ma to do siebie, że zwykle ma niewielkie warstwy dostępu do I/O (baza, http, kolejki, etc), a co za tym idzie łatwo jest stworzyć imitacje tych warstw i je używać w testach.

Nie sądzę, aby to miało znaczenie. Mikroserwis oznacza rozmiar w szerokości, a nie w głębokości czy w poziomie skomplikowania kodu.

Przykład z pierwszego postu jest trochę nieadekwatny, bo to jest coś, co akurat faktycznie ma zależności, więc można je wstrzyknąć, jeśli nam to coś daje. Czyli nie w celu bezsensownego testowania frameworka mockującego, tylko np. po to, aby kontener IoC opakował te klasy w interceptory obsługujące wyjątki.

Głównym problemem jest to, że wielu ludzi każdą pomocniczą klasę traktują jako zależność i chcą ją wstrzykiwać, nawet jeśli nie mają z tego żadnego zysku. Efektem jest tylko kupa problemów, np. z pisaniem testów (wspomniałem już o automatycznym mockowaniu, którego ludzie nie stosują?). Po spytaniu, czemu wszystko wstrzykują otrzymuję zazwyczaj odpowiedź, że to "general rule of programming" i żebym sobie książki poczytał.
Efekt jest taki, że kod wygląda, jakby autor miał płacone od linijki. A ja później jestem w stanie skasować 70% kodu zwiększając jednocześnie pokrycie rzeczywistymi testami dwukrotnie.

Drugi problem to to, że ludzie zmieniają kod biznesowy pod kod testów. Jestem tego przeciwnikiem. No, ale to wszystko wynika z TDD (Tutorial Driven Development). Ponieważ w tutorialu było napisane, że wszystko trzeba mockować i wszystko ma mieć intefejsy, to tak trzeba robić. No przecież nikt by nie napisał głupoty w internecie, prawda? ;)

Dodatkowym problemem jest to, że ludzie mają dziwne podejście do organizacji kodu w klasy. Wyobraźmy sobie takie zadanie - trzeba wczytać z bazy listę przelewów do wykonania i wygenerować na ich podstawie pliki eksportu do różnych banków. (Pliki tekstowe, struktura zależna od banku.)
Typowa implementacja będzie wyglądała tak:

  • ExportService, który w zależnościach przyjmie:
  • ITransfersRepository;
  • IBankTypeFactory;
    IBankTypeFactory będzie zwracało obiekty typu IBankTypeProcessor, które w swoich zależnościach z kolei przyjmą: IHeaderGenerator, ITransferGenerator i IFileSaver. Główna metoda w ExportService wczyta dane z repozytorium i w pętli będzie sprawdzało typ banku docelowego, tenże przekazany do fabryki spowoduje zwrócenie procesora dla danego banku, z odpowiednio wygenerowanymi generatorami nagłówka, przelewu, itd.

I już mamy drabinkę rzeczy do mockowania, fabrykowania i składania. Nie da się inaczej, bo taka jest architektura - nie przetestuje się jednostkowo PkoBpTypeProcessor, dopóki nie zamockujemy FileSavera (który w tym przypadku będzie zwracany przez jakiegoś obrzydliwego mocka jakiejś obrzydliwej fabryki).

Tymczasem, u mnie ExportService zawierałoby po prostu:

  • TransfersSource (prostą klasę opakowującą ORMa, bo źródłem danych i tak jest baza);
  • mapę bank -> BankTypeProcessor;
  • FileSaver.

Generatory nagłówków i pojedynczych przelewów każdy konkretny BankTypeProcessor utworzy sobie sam - bo to jest i tak coś, co ma jedną implementację, i tak przydatną tylko wewnątrz tej konkretnej klasy.
I napiszę sobie testy konkretnych BankTypeProcesorów - bo to jest logika, którą przetwarza wejście w wyjście, nie mająca żadnych zewnętrznych zależności, a potem test integracyjny całego serwisu. Jedyny interfejs, który może się tu pojawić to wynikający z implementacji wzorca strategia przez poszczególne procesory (żeby dało się je trzymać w słowniku, a serwis mógł po prostu zawołać metodę).

W skrócie:

  1. Zależność to jest coś, co łączy kod ze światem zewnętrznym - plikiem, bazą, siecią, piwnicą msma, albo innym modułem aplikacji. Nie każda klasa, do której delegujemy kawałek pracy jest zależnością, większość z nich to po prostu własność klasy wołającej; lokalna klasa pomocnicza. Tego się nie wstrzykuje, bo istnieje tylko ze względu na SRP; tylko po to, aby główna klasa nie miała 500 linii lecz 50.
  2. Interfejsy służą do: oddzielania modułów aplikacji i wydzielania zewnętrznych zależności (patrz punkt poprzedni), ewentualnie jako podstawa we wzorcach projektowych. Definiowanie interfejsu do każdej klasy, bo "tak wymaga framework IoC/mockujący/testujący" albo bo "tak było na blogu" to po prostu strata czasu i robienie burdelu w kodzie.
0
somekind napisał(a):

Moim zdaniem DDD z tymi swoimi encjami, które operują same na sobie jest przereklamowane. Ale to bez znaczenia, bo z DDD jest jak z Agile - nikt go w praktyce nie używa, mimo że wszyscy o tym gadają.

Moim zdaniem DDD wygląda na używalne, ale oczywiście po wywaleniu ORMów, baz danych w domenie i innych tego typu śmieci. Ale, w sumie nie znam się DDD nie było nigdy celem samym w sobie, ani zgodność. Natomiast czasem widzę, że dobijam się w kodzie do podobnych do DDD koncepcji.

0

@somekind wreszcie jakis porządny post ;)
Podoba mi się twój przykład bo pokazuje mniej więcej dylemat który miałem na myśli. Niemniej dla mnie nadal nie jest do końca jasne kiedy traktować coś jako zależność a kiedy jako własność danej klasy, szczególnie kiedy np. ten twój generator jest częścią większego procesu, który jest częścią większego procesu... I nagle okazuje się, że jednostkowo można przetestować tylko sam dół, bo potem im wyżej tym test bardziej puchnie żeby przygotować odpowiedni input.

Tu nie ma co testować jednostkowo, bo tu nie ma logiki. A skoro to jest coś na szczycie aplikacji, to i tak trzeba to przetestować integracyjnie. Testy jednostkowe niczego tu nie wniosą, właściwie przetestuje się tylko działanie frameworka mockującego.

Już drugi raz pada tutaj ten argument i moim zdaniem to nie do końca jest prawda. Logiki jest tam mało, to fakt, ale to nie znaczy ze w ogóle jej nie ma. Już teraz są tam potencjalnie 4 rzeczy do sprawdzenia:

  • Czy ktoś nie zapomniał odpalić filtrowania danych - cała reszta niejako z automatu jest sprawdzona przez type system, ale ten jeden krok nie. Jeśli ktoś przypadkiem zakomentuje filtrowanie to trudno to będzie zauważyć. I proszę darować sobie bujdy na temat wyczerpującego zestawu danych dla testu integracyjnego, bo taki magiczny zestaw który obejmie wszystkie możliwe przypadki, to jest marzenie ściętej głowy. W tym przypadku to nie problem, ale przy nieco bardziej skomplikowanym kodzie może to być nietrywialne.
  • Sytuacje wyjątkowe - szczególnie jeśli obsługa nie jest widoczna wyżej, tzn nie rzucamy nowego wyjątku tylko na przykład coś logujemy, albo generujemy event który wyświetli userowi komunikat błędu itd. Brak testu znów oznacza, że można popsuć implementacje i nikt tego nie zauważy. Przynajmniej do czasu kiedy ktoś zgłosi że funkcjonalność nie działa i nie jest to nijak sygnalizowane, okienek z błędem nie ma, w logach pusto itd ;] Nie wiem czy klient zgodzi się z argumentacją że "tego nie trzeba testować bo nie ma tam logiki" skoro z jego punktu widzenia aplikacja nie działa ;]

Jeśli chodzi o mapę bank -> BankTypeProcessor; to znów pojawia się dylemat - taką mapę można automatycznie wstrzyknąć sobie przez IoC!
Plus: nie ma ryzyka ze zapomnisz czegoś dodać do mapy bo IoC wrzuci wszystkie implemetnacje dostępne w runtime, nie trzeba modyfikować niczego kiedy dodajemy nową implemetnacje bo będzie dostępna automatycznie.
Minus: Nagle w kodzie dzieje sie coś czego na pierwszy rzut oka "nie widać", trochę jak w przypadku AOP. I moze tak być, że ktoś dopiero co zaczął implementacje jakiegoś BankTypeProcessora dla nowego banku, implemetnacja jeszcze nie skończona no ale jeszcze nigdzie nie jest użyta, leci commit i system się wywala bo IoC automatycznie dorzuciło tą klasę do dostępnych implementacji...

2
Shalom napisał(a):
  • Czy ktoś nie zapomniał odpalić filtrowania danych - cała reszta niejako z automatu jest sprawdzona przez type system, ale ten jeden krok nie.

No dobrze, ale jeżeli ten krok jest taki ważny, to robisz dostarczyciela danych, który ma filtrowanie i testujesz tego dostarczyciela.

Shalom napisał(a):

I proszę darować sobie bujdy na temat wyczerpującego zestawu danych dla testu integracyjnego, bo taki magiczny zestaw który obejmie wszystkie możliwe przypadki, to jest marzenie ściętej głowy.

Jasne, że nie obejmiesz, ale i przy teście integracyjnym nie to jest celem. Masz sprawdzić, czy system naprawdę działa, a logikę biznesową testujesz niżej.

Shalom napisał(a):
  • Sytuacje wyjątkowe - szczególnie jeśli obsługa nie jest widoczna wyżej, tzn nie rzucamy nowego wyjątku tylko na przykład coś logujemy, albo generujemy event który wyświetli userowi komunikat błędu itd.

Zgoda, że wypadałoby przetestować obsługę błędów (o czym pisałem też w poprzedniej wiadomości), ale od tego możesz mieć osobne klasy, które przetestujesz jednostkowo. Wtedy tutaj jest tylko przechwycenie wyjątku i wywołanie jakiejś metody — a wtedy pytanie brzmi, czy jest sens sprawdzać, czy metody zostały wywołane, będziesz robił mocki i robił Assert.MethodWasCalled?

Shalom napisał(a):

I moze tak być, że ktoś dopiero co zaczął implementacje jakiegoś BankTypeProcessora dla nowego banku, implemetnacja jeszcze nie skończona no ale jeszcze nigdzie nie jest użyta, leci commit i system się wywala bo IoC automatycznie dorzuciło tą klasę do dostępnych implementacji...

Jeżeli leci commit niedokończonego kodu do wspólnej gałęzi, to problemem nie jest ten commit, tylko proces w firmie. Gdzie code review? Gdzie testy? Dlaczego to w ogóle zostało wcommitowane? Chyba, że jest to commit do feature brancha, ale wtedy jak się system wywali, to nie ma problemu.

0
somekind napisał(a):
pingwindyktator napisał(a):

No i wspomniana już przez @Shalom wada DI - złożoność kodu. Żeby zmienić nasz MagicService na DI-friendly musimy dołożyć obrzydliwy, 6-argumentowy konstruktor, 6 interfejsów (które bardzo często mają tylko jedną implementację), pare fabryk i IoC. Też niefajne.

Sensowne kontenery IoC nie wymagają interfejsów do działania. Zresztą, nie spotkałem jeszcze nawet bezsensownego kontenera IoC, który by tego wymagał. :P
Sensowny kontener IoC nie wymaga też żadnych zmian w konfiguracji po zmianie sygnatury konstruktora.

Jak najbardziej masz rację, ale pojawia się tutaj problem testowania jednostkowego (co jest główną zaletą DI i zabawy z IoC). Chcąc zamockować obiekty, musimy trzymać je jako interfejs, coby podczas testów wrzucić na ten interfejs ów mocka. No przynajmniej w wielu językach tak jest.

2

Jasne, że nie obejmiesz, ale i przy teście integracyjnym nie to jest celem. Masz sprawdzić, czy system naprawdę działa, a logikę biznesową testujesz niżej.

Toż właśnie o tym mówie, że pojawiają sie tu głosy że tego to nie trzeba testować bo niewiele tu jest, integracyjnie sie to przeleci. Popatrz na przykład od @somekind - on tam właśnie sugeruje takie podejście. Poszczególne BankProcessory jednostkowo ale już cały ExportService tylko integracyjnie, co oznacza że najpewniej w tym ExportService będą nieprzetestowane ścieżki ;]

Wtedy tutaj jest tylko przechwycenie wyjątku i wywołanie jakiejś metody — a wtedy pytanie brzmi, czy jest sens sprawdzać, czy metody zostały wywołane, będziesz robił mocki i robił Assert.MethodWasCalled?

A czemu nie? Co jest w takim teście złego? Czy jeśli ktoś przypadkiem w trakcie refaktoringu nam naszego error handlera zakomentuje to test się wywali? Wywali się. Więc jak dla mnie spełnia swoją rolę.
Bo takie myślenie jakie sugeruje między innymi @jarekr000000 że nie ma co testować mocków jest moim zdaniem po prostu śmieszne. Ja rozumiem że robienie assertTrue(true); jest bez sensu, tak samo jak i sprawdzanie czy zaprogramowany mock zwrócił nam wartość którą zaprogramowaliśmy. Ale poza takimi idiotycznymi przypadkami nie widzę za bardzo gdzie jest problem w testowaniu z użyciem mocków, nawet jeśli weryfikujemy tylko czy jakaś metoda została zawołana. Czy nie wywołanie tej metody oznacza że ktoś zepsuł kod? Oznacza. Czy test się wywali? Wywali. Więc spełnił swoją rolę.
Ba, powiedziałbym nawet że test dla metody w stylu:

public A fun(A arg){
    return service.fun(arg);
}

Ma sens, bo nawet tak prostą funkcje można zepsuć przynajmniej na 2 sposoby:

  • przekazując zły parametr do serwisu
  • zwracając coś innego niż to zwrócił serwis

I dla mnie to nie jest "testowanie mocka". Test który sprawdzi czy nasz mockowany serwis dostał odpowiedni argument a następnie asercja czy wynik zwrócony przez testowaną metodę jest tym samym co zwrócił serwis jest zupełnie poprawnym i wartościowym testem, bo chroni nas przed tymi 2 potencjalnymi błędami wspomnianymi wyżej.
A zaraz ktoś wpadnie i będzie hurr durr przecież tu nie ma logiki to nie testujemy!. A jak jakiś śmieszek poza kontrolo zrobi tam nagle service.fun(null) albo lepiej service.fun(new A()) w przypływie geniuszu, a do tego przypadkiem test integracyjny będzie wysyłał pusty obiekt A (no bo przygotowanie pełnego zestawu danych jest trudne) to o błędzie dowiemy się bardzo bardzo późno, a powinniśmy już na etapie budowania zaraz po tym jak ktoś tą głupotę commitował.

Jeżeli leci commit niedokończonego kodu do wspólnej gałęzi, to problemem nie jest ten commit, tylko proces w firmie

Raczej smutna proza życia, szczególnie jeśli coś stoi jeszcze w SVNie ;) Zresztą czym innym jest commitowanie kodu który sie nie kompiluje albo wywala testy, a czym innym commitowanie czegoś co jest in progress, szczególnie jeśli nigdzie tego jeszcze nie używamy. Bo przecież generalna zasada jest taka, że trunk ma się zawsze poprawnie budować i powinien być stabilny. Jestem sobie w stanie wyobrazić sytuacje kiedy ktoś commituje jakąś niedokończoną implementacje (albo przynajmniej nieprzetestowaną jeszcze do końca), zakładając że nie wpłynie na stabilność systemu skoro nie jest używana.

1
Shalom napisał(a):

Toż właśnie o tym mówie, że pojawiają sie tu głosy że tego to nie trzeba testować bo niewiele tu jest, integracyjnie sie to przeleci. Popatrz na przykład od @somekind - on tam właśnie sugeruje takie podejście. Poszczególne BankProcessory jednostkowo ale już cały ExportService tylko integracyjnie, co oznacza że najpewniej w tym ExportService będą nieprzetestowane ścieżki ;]

Tak jak powiedziałem, jeżeli chcesz przetestować pechowe ścieżki, to nie ma problemu. Osobiście nie uważam tego za potrzebne, bo jeżeli ten kod tylko deleguje gdzie indziej, to CR powinno wyłapać większość błędów, ale jeżeli chcesz być pewien, to jak najbardziej napisz testy.

Shalom napisał(a):

A czemu nie? Co jest w takim teście złego? Czy jeśli ktoś przypadkiem w trakcie refaktoringu nam naszego error handlera zakomentuje to test się wywali? Wywali się. Więc jak dla mnie spełnia swoją rolę.

Jak ktoś przypadkiem zakomentuje kod, to CR powinno to wyłapać. Jeżeli chcesz sprawdzić, czy gdzieś jest wywołana metoda, to właśnie od tego masz testy integracyjne, które sprawdzają, czy system poprawnie spina moduły. Inna sprawa, jeżeli nie jesteś w stanie integracyjnie sprawdzić obsługi wyjątków, wtedy jestem w stanie zrozumieć sprawdzenie tego mockiem, ale nie wydaje mi się to eleganckie.

Shalom napisał(a):

Raczej smutna proza życia, szczególnie jeśli coś stoi jeszcze w SVNie ;) Zresztą czym innym jest commitowanie kodu który sie nie kompiluje albo wywala testy, a czym innym commitowanie czegoś co jest in progress, szczególnie jeśli nigdzie tego jeszcze nie używamy. Bo przecież generalna zasada jest taka, że trunk ma się zawsze poprawnie budować i powinien być stabilny. Jestem sobie w stanie wyobrazić sytuacje kiedy ktoś commituje jakąś niedokończoną implementacje (albo przynajmniej nieprzetestowaną jeszcze do końca), zakładając że nie wpłynie na stabilność systemu skoro nie jest używana.

A ja tego nie jestem w stanie zaakceptować (chociaż wiem, że i tak bywa). Wiem, że w korpożyciu jest różnie, ale nie przekonasz mnie, że commitowanie nietestowanego kodu bez CR jest okej. W piątek o 17 różne rzeczy wpadają do repozytorium, ale jak już staramy się robić porządnie, to możemy wystawić gita na komputerze jednego developera i mieć repozytorium w swoim zespole, mieć CR, mieć feature branche, a jednocześnie ciągle commitować do SVN-a.

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