Kod tylko do testów w kodzie produkcyjnym

Natknąłem się ostatnio na taki artykuł Testing Without Mocks: A Pattern Language

Artykuł dość długi, ale cała idea jest przedstawiona w sekcji Example. Z grubsza chodzi o to, żeby implementując infrastrukturę, napisać też implementację, która nie zależy od zewnętrznych narzędzi typu baza danych czy jakieś API. Taki "konfigurowalny" stub, ale to kod produkcyjny. Autor nazywa to Nullable. Potem w testach używamy tego Nullable zamiast mockować bazę, 3rd party API czy system plików.

Klepnąłem sobie prostą apkę z testami w takim stylu i muszę powiedzieć, że jest to całkiem przyjemne, ale strasznie mnie razi w oczy kod "tylko do testów" poplątany z kodem produkcyjnym. Już nie mówię nawet o tych Nullable'ach, ale to podejście wymaga czasem dodatkowych wywołań klas/metod, które służą tylko temu, żeby łatwiej (albo w ogóle) można było napisać asercję lub złapać wynik/efekt uboczny wywołania jakiejś funkcji. Przykład prosto od autora artykułu: implementacja CommandLine ma listener, który jest tam tylko po to, żeby potem w teście móc przechwycić output.

Po drugiej stronie spektrum jest podejście opisane tutaj (wybaczcie za film, nie mogę nigdzie tego znaleźć w formie tekstu). Tutaj z kolei autor poleca następujące podejście:

• zdefiniować zależności modułu (w OOP to będzie interface lub protocol),
• mockować zależności i sprawdzić, czy są dobrze wywoływane, np. jeśli testuję handler HTTP, to testuję, czy na GET /search&q=foobar wywołany zostanie SearchService.getResults("foobar"),
• stubować zależności i sprawdzić, czy handler HTTP jest w stanie obsłużyć wszystkie możliwe zwrotki od swoich zależności, np. stubuję SearchService tak, by getResults zwracało: pustą tablicę, jeden element, kilka elementów, dużo elementów i rzuciło wyjątek,
• moduł uznaję za przetestowany, a testując SearchService trzymam się zasady, że muszę napisać testy, które:
  • wywołują getResults z takimi parametrami, jakie sprawdzałem gdy mockowałem ten serwis w testach handlera HTTP, i osobno takie, które
  • robią setup w taki sposób, by getResults zwróciło mi wyniki, które zastubowałem testując handler HTTP

Po takich testach mam mieć pewność, że http handler poprawnie rozmawia ze swoimi zależnościami i potrafi obsłużyć to, co te zależności zwrócą, oraz, że service przyjmie i obsłuży to, co handler mu przekaże i w odpowiedniej sytuacji zwróci to, czego handler oczekuje.

Plus jest taki, że nie mieszam kodu produkcyjnego z testowym i nie muszę pisać dodatkowego kodu produkcyjnego tylko po to, żeby móc coś (łatwiej) przetestować. Minus jest taki, że jest dużo mocków, a mówią, że mocki są złe :)

Próbowałem po trochu obu podejść i o ile w drugim jest więcej klepania, mam wrażenie, że te testy są czytelniejsze, bardziej zwięzłe i nie testują tego samego 10 razy. Do tego łatwiej mi pisać aplikację "top-down", mogę się skupić na jednym module na raz, zależności zastubować/zamockować i martwić się nimi później. Jednakże używanie tylu mocków i stubów po prostu "doesn't feel right". Do tego nie ma narzędzi do sprawdzenia, czy wszystkie przypadki zakładane w testach handlera HTTP są przetestowane w testach serwisu, więc powiązanie jest dość luźne i wydaje się, że łatwo można popełnić błąd lub o czymś zapomnieć. Czy jest jakiś middleground między jednym a drugim, albo czy preferujecie jedno podejście od drugiego? Co sądzicie o samej idei przeplatania kodu produkcyjnego z testowym? Co sądzicie o pisaniu kodu produkcyjnego, który jest używany tylko w testach?

Według mnie kod nie powinień być konkretnie pod testy. Z drugiej strony kod powinień być napisany tak generycznie, żeby dało się go używać w różnych kontekstach tj. produkcyjne czy w testach.
Przypadek z listenerem jest IMO zły. Prawidłowo powinno się go przekazać przez konstrutkor (DI) w taki sposób, żeby nie potrzebne były takie pomocniczne metody.

Co do całości: po to masz interfejsy, żeby nie było konieczne mieszanie kodu produkcyjnego z testowym. Kod produkcyjny używa interfejsów w miejscach, gdzie chcesz mieć zmienne zachowanie i tyle. Idea przeplatania jest zła, bo cierpi na tym czytelność i utrzymanie. Jak dany kawałek kodu nie jest używany produkcyjnie to przyszły archeolog twojego kodu ma problem z ogarnięciem po cholerę to tutaj jest

Musiałeś to chyba źle zinterpretować, bo w sekcji "Nullable" nie ma nic o kodzie "tylko pod testy".

Jest tam tylko wstrzyknięta fabryka abstrakcyjna która zwraca albo prawdziwą instancję albo NullObject. Tzn, ten NullObject jest implementacją pod testy, ale to jest część samych testów, a nie kodu produkcyjnego.

Riddle napisał(a):

Musiałeś to chyba źle zinterpretować, bo w sekcji "Nullable" nie ma nic o kodzie "tylko pod testy".

Jest tam tylko wstrzyknięta fabryka abstrakcyjna która zwraca albo prawdziwą instancję albo NullObject. Tzn, ten NullObject jest implementacją pod testy, ale to jest część samych testów, a nie kodu produkcyjnego.

Zobacz na ten przykład https://github.com/jamesshore/testing-without-mocks-example/blob/javascript/src/infrastructure/command_line.js#L27 . Jest wystawiona statyczna metoda createNull, która wygląda jak kod tylko do testów.

slsy napisał(a):

Riddle napisał(a):

Musiałeś to chyba źle zinterpretować, bo w sekcji "Nullable" nie ma nic o kodzie "tylko pod testy".

Jest tam tylko wstrzyknięta fabryka abstrakcyjna która zwraca albo prawdziwą instancję albo NullObject. Tzn, ten NullObject jest implementacją pod testy, ale to jest część samych testów, a nie kodu produkcyjnego.

Zobacz na ten przykład https://github.com/jamesshore/testing-without-mocks-example/blob/javascript/src/infrastructure/command_line.js#L27 . Jest wystawiona statyczna metoda createNull, która wygląda jak kod tylko do testów.

No to zgadzam się, że faktycznie ten kod jest bez sensu.

Nie tak się powinno to zrobić.

Czyli jak dobrze rozumiem: stuby i mocki OK, ale w kodzie testowym, nie produkcyjnym?

Biorąc na tapet ten kawałek kodu konstruktor CommandLine powinien przyjmować jakiś interfejs, który byłby implementowany przez Node'owy process (albo nasz wrapper na process), a w testach... no właśnie. Jak przetestować, czy writeOutput robi to, co powinien? Czy process w testach powinien być zmockowany i sprawdzać, czy jest wywołane stdout.write z opowiednim parametrem? Czy może powinienem wstrzyknąć jakiś processSpy, który mi przechwyci wszystko, co wchodzi do stdout.write i zapisze w zmiennej, do której mam dostęp i mogę sobie napisać assert expectedOutput == processSpy.capturedOutput? W zasadzie to wiele się nie różni od mocka i assertCalledWith. Czy da się jakoś inaczej, najlepiej tak, by jak najmniej (w ogóle) nie wiązać testu z implementacją?

Ja bym to zrobił tak (pseudo-typescript):

//////////// kod produkcyjny
// command_line.ts
interface Process {
  argv(): string[];
  write(text: string);
}
class CommandLine {
  constructor(process: Process) {
     this.process = process;
  }

  args(): string[] {
    return this.process.argv().slice(2);
  }

  writeOutput(text: string) {
    this.process.write(text);
  }
}

// node_process.ts
class NodeProcess implements Process {
  argv(): string[] {
    return process.argv;
  }
  write(text: string) {
    process.stdout.write(text);
  }
}

//////////// main
// write passed args to stdout
let commandLine = CommandLine(NodeProcess());
commandLine.writeOutput(commandLine.args());

///////////// testy
// command_line_test.ts
class ProcessDouble implements Process {
  constructor(argv: string[]) {
    this.argv = argv;
    this.capturedOutput: string[] = [];
  }
  argv(): string[] {
    return this.argv;
  }
  write(text: string) {
    this.capturedOutput.push(text);
  }
  
}
describe('CommandLine', function() {
  it('gets args', function() {
    const args = ["foo", "bar"];
    let commandLine = CommandLine(ProcessDouble(["nodejs", "main", ...args]));
    assert.Equal(args, commandLine.args());
  });

  it('writes output', function() {
    const text = "hello";
    let process = ProcessDouble();
    CommandLine(process).write(text);
    assert.Equal(process.capturedOutput, [text]);
  });
})

// node_process_test.ts
// tutaj zrobiłbym test integracyjny, tj. odpaliłbym prawdziwy proces z parametrami i przechwytywałbym stdout

Czekam na krytykę :) tylko proszę bez nit-pickingu. Wiem, że ta klasa nie ma dużego sensu, ale wyobraźmy sobie, że zawiera inne metody, trochę bardziej rozbudowane niż przekazywanie parametrów do warstwy niżej. Rozmawiajmy tylko o podejściu.

Druga opcja to nie pisałbym ProcessDouble tylko stworzył mocka Process i

1. przy założeniu, że mock.argv() zwraca ["a", "b", "c"] napisałbym assert ["c"] == CommandLine(mock).args()
2. const text = "hello"; CommandLine(mock).write(text); assert.CalledWith(mock.write, text)

Zasadniczo nie widzę dużej różnicy między tymi podejściami. Jakie widzicie wady/zalety?

iksde napisał(a):`

Zasadniczo nie widzę dużej różnicy między tymi podejściami. Jakie widzicie wady/zalety?

Wygląda dobrze. Co do różnicy: praktycznie nie ma żadnej, mock (jak nie wołasz żadnego CalledWith i innych rzeczy sprawdzających co i ile razy zostało zawołane) to w gruncie rzeczy tworzenie takiego ProcessDouble tylko w mniej manualny sposób.

Co do assert.CalledWith(mock.write, text) to nie lubię używać takich instrukcji. Lepiej sprawdzać wynik a nie to, czy dana metoda została zawołana. Dużo lepiej napisać kod i testy w taki sposób, że w razie jakiejkolwiek zmiany (capturedOutput jest dobrym przykładem) możesz od razu wychwycić co się stało

slsy napisał(a):

Według mnie kod nie powinień być konkretnie pod testy. Z drugiej strony kod powinień być napisany tak generycznie, żeby dało się go używać w różnych kontekstach tj. produkcyjne czy w testach.
Przypadek z listenerem jest IMO zły. Prawidłowo powinno się go przekazać przez konstrutkor (DI) w taki sposób, żeby nie potrzebne były takie pomocniczne metody.

Co do całości: po to masz interfejsy, żeby nie było konieczne mieszanie kodu produkcyjnego z testowym. Kod produkcyjny używa interfejsów w miejscach, gdzie chcesz mieć zmienne zachowanie i tyle. Idea przeplatania jest zła, bo cierpi na tym czytelność i utrzymanie. Jak dany kawałek kodu nie jest używany produkcyjnie to przyszły archeolog twojego kodu ma problem z ogarnięciem po cholerę to tutaj jest

Też kiedyś tak uważałem, ale to wynikało trochę z niedostatków języków, w których pisałem. Tzn w wielu językach (np Java) podział pomiędzy kodem produkcyjnym a testowym może przebiegać tylko na granicy całych plików - tzn masz pewne pliki tylko testowe w osobnym katalogu (choć nawet mogą być w tym samym pakiecie). O tym czy plik jest włączony do ostatecznego buildu produkcyjnego decyduje system budowania projektu a nie kompilator.

Tymczasem da się znacznie lepiej. Istnieją języki obsługujące kompilację warunkową (C, C++, Rust, D) i tam można sobie dowolny fragment kodu, nawet jedną linijkę oznaczyć jako do testów. Wtedy nie ma pytania "po cholerę jest ten kod", bo widzisz czarno na białym #[cfg(test)]. Oczywiście realnie nikt chyba nie używa tego do przeplatania kodu co linijkę, ale jest to użyteczne np. jak musisz w testach dobrać się do prywatnych bebechów jakiegoś modułu - wtedy możesz to zrobić bez psucia API modułu dodając np. testowy getter czy dodatkowe testowe metody walidujace stan obiektu.
Dzięki temu nie komplikujesz niepotrzebnie kodu produkcyjnego tylko po to aby napisać testy.

W kodzie produkcyjnym nie powinno być nic pod testy. Jeżeli jest, to najprawdopodobniej testujesz implementację a nie funkcjonalność jednostek.

piotrpo napisał(a):

W kodzie produkcyjnym nie powinno być nic pod testy. Jeżeli jest, to najprawdopodobniej testujesz implementację a nie funkcjonalność jednostek.

Nie powinno się testować implementacji, zgadzam się, ale rozważany tutaj przypadek bynajmniej tego nie robi i autor to kilkakrotnie zaznacza, m.in tutaj:

Easy refactoring. Object interactions are considered implementation to be encapsulated, not behavior to be tested. Although the consequences of object interactions are tested, the specific method calls aren’t. This allows structural refactorings to be made without breaking tests.

Readable tests. Tests follow a straightforward “arrange, act, assert” structure. They describe the externally-visible behavior of the unit under test, not its implementation. They can act as documentation for the unit under test.

Pokazuje też kilka wzorców, które mają jeszcze bardziej odizolować testy od implementacji, albo przynajmniej ułatwić refaktoryzację.