Czy modyfikowanie kodu wyłącznie do celów testowych jest złą praktyką?

Prowadzę debatę z kolegą programistą na temat tego, czy dobrą lub złą praktyką jest modyfikowanie działającego kodu tylko w celu umożliwienia jego przetestowania (na przykład poprzez testy jednostkowe).

Moim zdaniem jest w porządku, oczywiście w granicach utrzymywania dobrych praktyk obiektowych i inżynierii oprogramowania (nie „upubliczniania wszystkiego” itp.).

Zdaniem mojego kolegi modyfikowanie kodu (który działa) tylko do celów testowych jest błędne.

Prosty przykład, pomyśl o tym fragmencie kodu, który jest używany przez jakiś komponent (napisany w C #):

public void DoSomethingOnAllTypes()
{
    var types = Assembly.GetExecutingAssembly().GetTypes();

    foreach (var currentType in types)
    {
        // do something with this type (e.g: read it's attributes, process, etc).
    }
}

Zasugerowałem, że ten kod można zmodyfikować, aby wywołać inną metodę, która wykona rzeczywistą pracę:

public void DoSomething(Assembly asm)
{
    // not relying on Assembly.GetExecutingAssembly() anymore...
}

Ta metoda pobiera obiekt asemblera do pracy, umożliwiając przekazanie własnego asemblera do przeprowadzenia testu. Mój kolega nie uważał, że to dobra praktyka.

Co jest uważane za dobrą i powszechną praktykę?

Modyfikowanie kodu, aby uczynić go bardziej testowalnym, ma więcej zalet niż testowalność. Ogólnie kod, który jest bardziej testowalny

• Jest łatwiejszy w utrzymaniu,
• Łatwiej jest uzasadnić,
• Jest luźniej sprzężony i
• Ma lepszy ogólny projekt pod względem architektonicznym.

W grze występują (pozornie) przeciwne siły .

• Z jednej strony chcesz wymusić enkapsulację
• Z drugiej strony chcesz móc testować oprogramowanie

Zwolennicy zachowania poufności wszystkich „szczegółów implementacji” są zwykle motywowani chęcią zachowania enkapsulacji. Jednak trzymanie wszystkiego w zamknięciu i niedostępne jest niezrozumiałym podejściem do enkapsulacji. Jeśli ostatecznym celem było pozostawienie wszystkiego niedostępnym, jedynym prawdziwym kodowanym kodem byłby:

static void Main(string[] args)

Czy twój kolega proponuje uczynienie z tego jedynego punktu dostępu w twoim kodzie? Czy wszystkie inne kody powinny być niedostępne dla dzwoniących z zewnątrz?

Ledwie. Dlaczego zatem można upublicznić niektóre metody? Czy nie jest to w końcu subiektywna decyzja projektowa?

Nie do końca. Tym, co prowadzi programistów, nawet na poziomie nieświadomości, jest znowu koncepcja enkapsulacji. Czujesz się bezpiecznie, wystawiając metodę publiczną, gdy odpowiednio chroni niezmienniki .

Nie chciałbym, aby odsłonić prywatną metodę, która nie chroni jego niezmienniki, ale często można go zmodyfikować tak, że ma chronić jego niezmienniki, a następnie wystawiać je publicznie (oczywiście z TDD, robisz to na inna droga naokoło).

Otwarcie API dla testowalności jest dobrą rzeczą , ponieważ tak naprawdę robisz to stosując zasadę Open / Closed .

Jeśli masz tylko jednego wywołującego interfejs API, nie wiesz, jak elastyczny jest naprawdę interfejs API. Możliwe, że jest dość mało elastyczny. Testy działają jak drugi klient, dając cenne informacje zwrotne na temat elastyczności interfejsu API .

Dlatego jeśli testy sugerują, że należy otworzyć interfejs API, zrób to; ale zachowaj enkapsulację, nie poprzez ukrywanie złożoności, ale przez ujawnianie złożoności w sposób bezpieczny w razie awarii.

Wygląda na to, że mówisz o iniekcji zależności . Jest to naprawdę powszechne i IMO, bardzo potrzebne do testowalności.

Aby odpowiedzieć na szersze pytanie, czy dobrym pomysłem jest modyfikowanie kodu tylko po to, aby był testowalny, pomyśl o tym w ten sposób: kod ma wiele obowiązków, w tym a) do wykonania, b) do odczytania przez ludzi, oraz c) do być przetestowanym. Wszystkie trzy są ważne, a jeśli twój kod nie spełnia wszystkich trzech obowiązków, powiedziałbym, że nie jest to bardzo dobry kod. Więc zmodyfikuj!

To trochę problem z jajkiem i kurczakiem.

Jednym z głównych powodów, dla których warto mieć dobry zasięg testowy kodu, jest to, że pozwala on na refaktoryzację bez strachu. Ale jesteś w sytuacji, w której musisz zmienić kod, aby uzyskać dobry zasięg testu! A twój kolega boi się.

Widzę punkt widzenia twojego kolegi. Masz kod, który (prawdopodobnie) działa, a jeśli pójdziesz i go poprawisz - z jakiegokolwiek powodu - istnieje ryzyko, że go złamiesz.

Ale jeśli jest to kod, który powinien podlegać ciągłej konserwacji i modyfikacjom, będziesz narażać się na to ryzyko za każdym razem, gdy będziesz nad nim pracował. A refaktoryzacja teraz i uzyskanie teraz pokrycia testowego pozwoli ci podjąć to ryzyko, w kontrolowanych warunkach, i nadać kodowi lepszą formę do przyszłych modyfikacji.

Powiedziałbym więc, że jeśli ta konkretna baza kodu nie jest dość statyczna i nie oczekuje się od niej znacznej pracy w przyszłości, to, co chcesz zrobić, to dobra praktyka techniczna.

Oczywiście, czy jest to dobra praktyka biznesowa, to cała „puszka robaków”.

Może to być tylko różnica w podkreśleniu w stosunku do innych odpowiedzi, ale powiedziałbym, że kod nie powinien być refaktoryzowany wyłącznie w celu poprawy testowalności. Testowalność jest bardzo ważna dla konserwacji, ale testowalność nie jest celem samym w sobie. W związku z tym odracza wszelkie takie refaktoryzacje, dopóki nie będziesz mógł przewidzieć, że ten kod będzie wymagał konserwacji w celu dalszego zakończenia działalności.

W momencie, że okaże się, że kod ten będzie wymagał pewne utrzymanie, że byłby to dobry czas byłaby dla sprawdzalności. W zależności od twojego uzasadnienia biznesowego może być słusznym założeniem, że cały kod będzie w końcu wymagał konserwacji, w którym to przypadku znika różnica z innymi odpowiedziami ( np . Odpowiedź Jasona Swetta ).

Podsumowując: sama testowalność nie jest (IMO) wystarczającym powodem do refaktoryzacji podstawy kodu. Testowalność odgrywa ważną rolę w umożliwieniu konserwacji na podstawie kodu, ale jest to wymóg biznesowy, aby zmodyfikować funkcję kodu, która powinna prowadzić do refaktoryzacji. Jeśli nie ma takich wymagań biznesowych, prawdopodobnie lepiej byłoby pracować nad czymś, na czym będą dbać Twoi klienci.

(Oczywiście nowy kod jest aktywnie utrzymywany, dlatego należy go napisać, aby był testowalny).

Myślę, że twój kolega się myli.

Inni wymieniali powody, dla których jest to już dobra rzecz, ale tak długo, jak masz do zrobienia, powinieneś być w porządku.

Powodem tego jest to, że wprowadzenie jakiejkolwiek zmiany w kodzie wiąże się z koniecznością ponownego przetestowania kodu. W zależności od tego, co robisz, ta praca testowa może być sama w sobie dużym wysiłkiem.

Podejmowanie decyzji o refaktoryzacji niekoniecznie jest twoim miejscem, a pracą nad nowymi funkcjami, które przyniosą korzyści Twojej firmie / klientowi.

Użyłem narzędzi pokrycia kodu jako części testów jednostkowych w celu sprawdzenia, czy wszystkie ścieżki w kodzie są wykonywane. Jako całkiem dobry program do kodowania / testowania, zwykle pokrywam 80-90% ścieżek kodu.

Kiedy badam odkryte ścieżki i staram się znaleźć niektóre z nich, wtedy odkrywam błędy, takie jak przypadki błędów, które „nigdy się nie zdarzają”. Tak, modyfikacja kodu i sprawdzenie zasięgu testu zapewnia lepszy kod.

Twój problem polega na tym, że twoje narzędzia testowe są gówniane. Powinieneś być w stanie wykpić ten obiekt i wywołać metodę testową bez zmiany - ponieważ ten prosty przykład jest naprawdę prosty i łatwy do modyfikacji, co dzieje się, gdy masz coś o wiele bardziej skomplikowanego.

Wiele osób zmodyfikowało swój kod, aby wprowadzić klasy IoC, DI i klasy oparte na interfejsie, aby umożliwić testowanie jednostkowe przy użyciu narzędzi do kpowania i testowania jednostkowego, które wymagają tych zmian kodu. Nie myślę, że to zdrowa rzecz, nie kiedy widzisz, że kod, który był dość prosty i prosty, zamienił się w koszmarny bałagan złożonych interakcji napędzany całkowicie potrzebą uczynienia każdej metody klasy całkowicie oddzieloną od wszystkiego innego . Aby dodać obrażenia do obrażeń, mamy wiele argumentów na temat tego, czy prywatne metody powinny być testowane jednostkowo, czy nie! (oczywiście, że powinni

Problem oczywiście tkwi w charakterze oprzyrządowania testowego.

Dostępne są teraz lepsze narzędzia, które mogą na zawsze wprowadzić te zmiany w projekcie do łóżka. Microsoft ma podróbki (z wyjątkiem Moli), które pozwalają na wycieranie konkretnych obiektów, w tym statycznych, więc nie trzeba już zmieniać kodu, aby pasował do narzędzia. W twoim przypadku, jeśli użyjesz podróbek, zastąpisz wywołanie GetTypes swoim własnym, które zwróciło prawidłowe i nieprawidłowe dane testowe - co jest dość ważne, sugerowana zmiana w ogóle tego nie przewiduje.

Aby odpowiedzieć: Twój kolega ma rację, ale być może z niewłaściwych powodów. Nie zmieniaj kodu do testowania, zmieniaj narzędzie testowe (lub całą strategię testowania, aby mieć więcej testów jednostkowych w stylu integracji zamiast takich szczegółowych testów).

Martin Fowler ma dyskusję na ten temat w swoim artykule Mocks are not Stubs

Dobrą i powszechną praktyką jest używanie dzienników testów jednostkowych i debugowania . Testy jednostkowe zapewniają, że jeśli dokonasz jakichkolwiek dalszych zmian w programie, twoja stara funkcjonalność się nie psuje. Dzienniki debugowania mogą pomóc w śledzeniu programu w czasie wykonywania.
Czasami zdarza się, że nawet poza tym musimy mieć coś tylko do celów testowych. Zmiana tego kodu nie jest niczym niezwykłym. Należy jednak zachować ostrożność, aby z tego powodu nie wpływać na kod produkcyjny. W C ++ i C można to osiągnąć za pomocą MAKRO , który jest jednostką czasu kompilacji. Wówczas kod testowy w ogóle nie pojawia się w środowisku produkcyjnym. Nie wiem, czy taki przepis jest w języku C #.
Również po dodaniu kodu testowego do programu powinien on być wyraźnie widocznyże ta część kodu jest dodawana w celach testowych. Albo programiści próbujący zrozumieć kod po prostu pocą się nad tą częścią kodu.

Istnieją poważne różnice między przykładami. W przypadku DoSomethingOnAllTypes()istnieje implikacja, która do somethingma zastosowanie do typów w bieżącym zespole. Ale DoSomething(Assembly asm)wyraźnie wskazuje, że możesz przekazać do niej dowolny zestaw.

Powodem, dla którego zwracam na to uwagę, jest to, że wiele kroków polegających wyłącznie na wstrzykiwaniu zależności zależy od oryginalnego obiektu. Wiem, że powiedziałeś „ nie„ upubliczniać wszystkiego ” ”, ale jest to jeden z największych błędów tego modelu, a zaraz po nim: otwarcie metod obiektu na zastosowania, do których nie są przeznaczone.

Twoje pytanie nie zawierało wiele kontekstu, w którym argumentował twój kolega, więc jest miejsce na spekulacje

„zła praktyka” zależy od tego, jak i kiedy zostaną wprowadzone zmiany.

W mojej opinii twój przykład wyodrębnienia metody DoSomething(types)jest w porządku.

Ale widziałem kod, który nie jest w porządku tak:

public void DoSomethingOnAllTypes()
{
  var types = (!testmode) 
      ? Assembly.GetExecutingAssembly().GetTypes() 
      : getTypesFromTestgenerator();

  foreach (var currentType in types)
  {
     if (!testmode)
     {
        // do something with this type that made the unittest fail and should be skipped.
     }
     // do something with this type (e.g: read it's attributes, process, etc).
  }
}

Te zmiany sprawiły, że kod stał się trudniejszy do zrozumienia, ponieważ zwiększyłeś liczbę możliwych ścieżek kodu.

Co mam na myśli mówiąc o tym, jak i kiedy :

jeśli masz działającą implementację i ze względu na „implementację możliwości testowania” dokonałeś zmian, musisz ponownie przetestować aplikację, ponieważ być może złamałeś DoSomething()metodę.

if (!testmode)Więcej więcej difficulilt zrozumieć i testowanego związku niż wyekstrahowanego metodą.