Czy konstruowanie obiektów o zerowych parametrach w testach jednostkowych jest prawidłowe?

37

Zacząłem pisać testy jednostkowe dla mojego obecnego projektu. Ale tak naprawdę nie mam z tym doświadczenia. Najpierw chcę całkowicie go „zdobyć”, więc obecnie nie używam ani mojego środowiska IoC, ani fałszywej biblioteki.

Zastanawiałem się, czy jest coś złego w podawaniu zerowych argumentów konstruktorom obiektów w testach jednostkowych. Pozwól mi podać przykładowy kod:

public class CarRadio
{...}


public class Motor
{
    public void SetSpeed(float speed){...}
}


public class Car
{
    public Car(CarRadio carRadio, Motor motor){...}
}


public class SpeedLimit
{
    public bool IsViolatedBy(Car car){...}
}

Jeszcze inny przykład kodu samochodu (TM), zredukowany do tylko części ważnych dla pytania. Napisałem test coś takiego:

public class SpeedLimitTest
{
    public void TestSpeedLimit()
    {
        Motor motor = new Motor();
        motor.SetSpeed(10f);
        Car car = new Car(null, motor);

        SpeedLimit speedLimit = new SpeedLimit();
        Assert.IsTrue(speedLimit.IsViolatedBy(car));
    }
}

Test działa dobrze. SpeedLimitpotrzebuje Carz Motorcelem, co robi. Nie jest wcale zainteresowany CarRadio, więc podałem dla tego wartość zerową.

Zastanawiam się, czy obiekt zapewniający poprawną funkcjonalność bez pełnej budowy jest naruszeniem SRP lub zapachem kodu. Mam to dokuczliwe wrażenie, że tak jest, ale speedLimit.IsViolatedBy(motor)też nie jest w porządku - ograniczenie prędkości jest naruszane przez samochód, a nie przez silnik. Może po prostu potrzebuję innej perspektywy dla testów jednostkowych niż działającego kodu, ponieważ cała intencja polega na przetestowaniu tylko części całości.

Czy konstruowanie obiektów z zerowym testem jednostkowym ma zapach kodu?

R. Schmitz
źródło
24
Trochę nie na temat, ale Motorprawdopodobnie nie powinien mieć speedwcale. Powinien mieć throttlei obliczyć na torquepodstawie bieżącej rpmi throttle. Zadaniem samochodu jest wykorzystanie go Transmissiondo zintegrowania tego z aktualną prędkością i przekształcenie go w rpmsygnał z powrotem do Motor... Ale wydaje mi się, że i tak nie byłeś zaangażowany w realizm, prawda?
cmaster
17
Zapach kodu polega na tym, że twój konstruktor zajmuje zero, bez narzekania. Jeśli uważasz, że jest to do przyjęcia (możesz mieć swoje powody): śmiało, przetestuj to!
Tommy
4
Rozważ wzorzec Null-Object . Często upraszcza kod, jednocześnie czyniąc go bezpiecznym dla NPE.
Bakuriu
17
Gratulacje : przetestowałeś to za pomocą nullradia, ograniczenie prędkości jest poprawnie obliczone. Teraz możesz utworzyć test, aby sprawdzić ograniczenie prędkości za pomocą radia; na wypadek, gdyby zachowanie się różniło ...
Matthieu M.
3
Nie jestem pewien co do tego przykładu, ale czasami fakt, że chcesz przetestować tylko pół klasy, jest wskazówką, że coś powinno zostać zerwane
Owen

Odpowiedzi:

70

W przypadku powyższego przykładu uzasadnione jest, że a Carmoże istnieć bez CarRadio. W takim przypadku powiedziałbym, że nie tylko CarRadioczasami dopuszczalne jest wprowadzenie wartości zerowej , ale także to, że jest to obowiązkowe. Twoje testy muszą zapewnić, że implementacja Carklasy jest niezawodna i nie zgłasza wyjątków zerowego wskaźnika, gdy nie CarRadiowystępuje.

Weźmy jednak inny przykład - rozważmy SteeringWheel. Załóżmy, że a Carmusi mieć SteeringWheel, ale test prędkości tak naprawdę nie obchodzi go. W tym przypadku nie dałbym wartości zerowej, SteeringWheelponieważ popycha to Carklasę w miejsca, do których nie jest przeznaczona. W takim przypadku lepiej byłoby stworzyć coś, DefaultSteeringWheelco (aby kontynuować metaforę) jest zablokowane w linii prostej.

Pete
źródło
44
I nie zapomnij napisać testu jednostkowego, aby sprawdzić, Carczy nie da się zbudować bez SteeringWheel...
cmaster
13
Być może czegoś mi brakuje, ale jeśli jest to możliwe, a nawet wspólne tworzenie Carbez CarRadio, nie ma sensu tworzyć konstruktora, który nie wymaga przekazywania i nie musi martwić się wyraźnym zeriem ?
earwig
16
Samochody samobieżne mogą nie mieć kierownicy. Tylko mówię. ☺
BlackJack
1
@James_Parsons Zapomniałem dodać, że chodziło o klasę, która nie miała kontroli zerowej, ponieważ była używana tylko wewnętrznie i zawsze otrzymywała parametry inne niż null. Inne metody Carwyrzuciłyby NRE z wartością zerową CarRadio, ale odpowiedni kod SpeedLimitnie. W przypadku zamieszczonego teraz pytania byłoby poprawne i rzeczywiście to zrobiłbym.
R. Schmitz
2
@mtraceur Sposób, w jaki wszyscy zakładali, że klasa zezwalająca na konstrukcję z argumentem zerowym oznacza, że ​​null jest prawidłową opcją, uświadomił mi, że prawdopodobnie powinienem tam mieć sprawdzanie zerowe. Rzeczywisty problem, który miałbym wtedy, obejmuje wiele dobrych, interesujących, dobrze napisanych odpowiedzi, których nie chcę zrujnować i które mi pomogły. Akceptuję teraz również tę odpowiedź, ponieważ nie lubię pozostawiać rzeczy niedokończonych i jest to najbardziej pozytywna opinia (chociaż wszystkie były pomocne).
R. Schmitz
23

Czy konstruowanie obiektów z zerowym testem jednostkowym ma zapach kodu?

Tak. Zwróć uwagę na definicję zapachu kodu C2 : „CodeSmell to wskazówka, że ​​coś może być nie tak, a nie pewność”.

Test działa dobrze. SpeedLimit potrzebuje samochodu z silnikiem, aby móc to zrobić. CarRadio w ogóle nie jest zainteresowany, więc podałem dla tego wartość zerową.

Jeśli próbujesz wykazać, że speedLimit.IsViolatedBy(car)nie zależy to od CarRadioargumentu przekazanego konstruktorowi, prawdopodobnie przyszłym opiekunem byłoby wyraźniejsze określenie tego ograniczenia poprzez wprowadzenie podwójnego testu, który nie przejdzie testu, jeśli zostanie wywołany.

Jeśli, co jest bardziej prawdopodobne, po prostu próbujesz zaoszczędzić sobie pracy nad stworzeniem, o CarRadioktórym wiesz, że nie jest używane w tym przypadku, powinieneś zauważyć, że

  • jest to naruszenie enkapsulacji (pozwalasz, aby CarRadionieużywany wyciekł do testu)
  • odkryłeś co najmniej jeden przypadek, w którym chcesz utworzyć Carbez określaniaCarRadio

Podejrzewam, że kod próbuje ci powiedzieć, że potrzebujesz konstruktora, który wygląda

public Car(IMotor motor) {...}

a następnie ten konstruktor może zdecydować, co zrobić z faktem, że nie maCarRadio

public Car(IMotor motor) {
    this(null, motor);
}

lub

public Car(IMotor motor) {
    this( new ICarRadio() {...} , motor);
}

lub

public Car(IMotor motor) {
    this( new DefaultRadio(), motor);
}

itp.

Chodzi o przekazanie wartości null do czegoś innego podczas testowania SpeedLimit. Widać, że test nazywa się „SpeedLimitTest”, a jedynym Assertem jest sprawdzanie metody SpeedLimit.

To drugi interesujący zapach - aby przetestować SpeedLimit, musisz zbudować cały samochód wokół radia, nawet jeśli jedyną rzeczą, na której prawdopodobnie zależy SpeedLimit, jest prędkość .

Może to być wskazówka, która SpeedLimit.isViolatedBypowinna akceptować interfejs roli, który implementuje samochód , zamiast wymagać całego samochodu.

interface IHaveSpeed {
    float speed();
}

class Car implements IHaveSpeed {...}

IHaveSpeedto naprawdę kiepskie imię; mam nadzieję, że Twój język domeny ulegnie poprawie.

Przy takim interfejsie test na SpeedLimitmoże być znacznie prostszy

public void TestSpeedLimit()
{
    IHaveSpeed somethingTravelingQuickly = new IHaveSpeed {
        float speed() { return 10f; }
    }

    SpeedLimit speedLimit = new SpeedLimit();
    Assert.IsTrue(speedLimit.IsViolatedBy(somethingTravelingQuickly));
}
VoiceOfUnreason
źródło
W ostatnim przykładzie zakładam, że musiałbyś stworzyć próbną klasę, która implementuje IHaveSpeedinterfejs, ponieważ (przynajmniej w c #) nie byłbyś w stanie utworzyć samego interfejsu.
Ryan Searle
1
Zgadza się - efektywna pisownia będzie zależeć od tego, jakiego smaku Blub używasz do swoich testów.
VoiceOfUnreason
18

Czy konstruowanie obiektów z zerowym testem jednostkowym ma zapach kodu?

Nie

Ani trochę. Przeciwnie, jeśli NULLwartość jest dostępna jako argument (niektóre języki mogą mieć konstrukcje uniemożliwiające przekazanie wskaźnika zerowego), należy ją przetestować.

Kolejne pytanie dotyczy tego, co dzieje się po zdaniu NULL. Ale właśnie o to chodzi w teście jednostkowym: upewnienie się, że dla każdego możliwego wejścia dochodzi do określonego wyniku. I NULL jest potencjalne wejście, więc trzeba mieć określony rezultat i trzeba mieć test na to.

Więc jeśli twój samochód ma być budowany bez radia, powinieneś napisać test na to. Jeśli nie jest i ma rzucić wyjątek, powinieneś napisać test na to.

Tak czy inaczej, tak, powinieneś napisać test dla NULL. Byłby to zapach kodu, gdybyś go pominął. Oznaczałoby to, że masz nieprzetestowaną gałąź kodu, która, jak zakładałeś, będzie magicznie wolna od błędów.


Pamiętaj, że zgadzam się z innymi odpowiedziami, że Twój testowany kod może zostać ulepszony. Niemniej jednak, jeśli ulepszony kod ma parametry, które są wskaźnikami, przekazywanie NULLnawet dla ulepszonego kodu jest dobrym testem. Chciałbym, aby test pokazał, co się stanie, gdy przejdę NULLjako silnik. To nie jest zapach kodu, to przeciwieństwo zapachu kodu. To jest testowanie.

nvoigt
źródło
Wygląda na to, że piszesz Cartutaj o testowaniu . Chociaż nie widzę niczego, co uważałbym za złe stwierdzenie, pytanie dotyczy testowania SpeedLimit.
R. Schmitz
@ R.Schmitz Nie jestem pewien, co masz na myśli. Cytuję pytanie, chodzi o przekazanie NULLkonstruktora Car.
nvoigt
1
Chodzi o przekazanie wartości zerowej do czegoś innego podczas testówSpeedLimit . Możesz zobaczyć, że test nazywa się „SpeedLimitTest”, a jedyną Assertjest sprawdzenie metody SpeedLimit. Testowanie Car- na przykład „jeśli Twój samochód ma być budowany bez radia, powinieneś napisać test na to” - odbyłby się w innym teście.
R. Schmitz,
7

Ja osobiście wahałbym się przed wprowadzeniem zer. W rzeczywistości za każdym razem, gdy wstrzykiwam zależności do konstruktora, jedną z pierwszych rzeczy, które robię, jest zgłaszanie wyjątku ArgumentNullException, jeśli te wstrzyknięcia są zerowe. W ten sposób mogę bezpiecznie korzystać z nich w mojej klasie, bez dziesiątek kontroli zerowych. Oto bardzo powszechny wzór. Zwróć uwagę na użycie tylko do odczytu, aby upewnić się, że te zależności ustawione w konstruktorze nie mogą być później ustawione na null (lub cokolwiek innego):

class Car
{
   private readonly ICarRadio _carRadio;
   private readonly IMotor _motor;

   public Car(ICarRadio carRadio, IMotor motor)
   {
      if (carRadio == null)
         throw new ArgumentNullException(nameof(carRadio));

      if (motor == null)
         throw new ArgumentNullException(nameof(motor));

      _carRadio = carRadio;
      _motor = motor;
   }
}

W związku z powyższym może być może przeprowadzę test jednostkowy lub dwa, w których wstrzykiwam wartości zerowe, aby upewnić się, że moje testy zerowe działają zgodnie z oczekiwaniami.

Powiedziawszy to, staje się to problemem, gdy zrobisz dwie rzeczy:

  1. Programuj interfejsy zamiast klas.
  2. Użyj kpiącego frameworka (takiego jak Moq dla C #), aby wstrzyknąć fałszywe zależności zamiast wartości zerowych.

Na przykład masz:

interface ICarRadio
{
   bool Tune(float frequency);
}

interface Motor
{
   bool SetSpeed(float speed);
}

...
var car = new Car(new Moq<ICarRadio>().Object, new Moq<IMotor>().Object);

Więcej informacji na temat korzystania z Moq można znaleźć tutaj .

Oczywiście, jeśli chcesz to zrozumieć bez kpin, najpierw możesz to zrobić, o ile korzystasz z interfejsów. Po prostu utwórz manekina CarRadio i Motor w następujący sposób i wstrzyknij je:

class DummyCarRadio : ICarRadio
{
   ...
}
class DummyMotor : IMotor
{
   ...
}

...
var car = new Car(new DummyCarRadio(), new DummyMotor())
Eternal21
źródło
3
Oto odpowiedź, którą bym napisał
Liath
1
Chciałbym nawet posunąć się do stwierdzenia, że ​​obojętne przedmioty również muszą wypełnić swój kontrakt. Gdyby IMotormiał getSpeed()metodę, DummyMotormusiałby również zwrócić zmodyfikowaną prędkość po udanym setSpeed.
ComFreek
1

To nie jest po prostu w porządku, jest to dobrze znana technika przełamywania zależności w celu wprowadzenia starszego kodu do testowej uprzęży. (Zobacz „Efektywna praca ze starszym kodem” autorstwa Michaela Feathersa.)

Czy to pachnie Dobrze...

Testu nie pachnie. Test wykonuje swoją pracę. Deklaruje „ten obiekt może być pusty, a testowany kod nadal działa poprawnie”.

Zapach jest w trakcie realizacji. Deklarując argument konstruktora, deklarujesz „ta klasa potrzebuje tego do prawidłowego funkcjonowania”. Oczywiście to nieprawda. Wszystko, co jest nieprawdą, jest trochę śmierdzące.

Gumowa kaczuszka
źródło
1

Cofnijmy się do pierwszych zasad.

Test jednostkowy sprawdza niektóre aspekty jednej klasy.

Będą jednak konstruktory lub metody, które przyjmują inne klasy jako parametry. Sposób ich obsługi różni się w zależności od przypadku, ale konfiguracja powinna być minimalna, ponieważ są one styczne do celu. Mogą istnieć scenariusze, w których podanie parametru NULL jest całkowicie poprawne - na przykład samochód bez radia.

Jestem zakładając tu, że jesteśmy po prostu testuje linię wyścigową zacząć (aby kontynuować temat samochodów), ale może też chcesz przekazać NULL do innych parametrów, aby sprawdzić, czy wszystkie mechanizmy obronne i kod wyjątku przekazanie funkcjonują jako wymagany.

Jak na razie dobrze. Co jednak, jeśli NULL nie jest prawidłową wartością. Cóż, tutaj jesteś w mrocznym świecie kpiny, stuby, manekiny i podróbki .

Jeśli wydaje się, że to wszystko wymaga trochę więcej, to już używasz manekina, przekazując NULL w konstruktorze Car, ponieważ jest to wartość, która potencjalnie nie zostanie użyta.

To, co powinno stać się jasne dość szybko, to to, że potencjalnie strzelasz do kodu z dużą ilością obsługi NULL. Jeśli zamienisz parametry klasy na interfejsy, utorujesz również drogę do wyśmiewania się, DI itp., Które znacznie ułatwiają obsługę.

Robbie Dee
źródło
1
„Testy jednostkowe służą do testowania kodu jednej klasy”. Westchnienie . Nie takie są testy jednostkowe, a przestrzeganie tych wytycznych poprowadzi cię do rozdętych klas lub do testów przynoszących efekt przeciwny do zamierzonego.
Ant P
3
traktowanie „jednostki” jako eufemizmu dla „klasy” jest niestety bardzo powszechnym sposobem myślenia, ale w rzeczywistości wszystko to (a) zachęca do testów „wrzucania, wyrzucania”, które mogą całkowicie podważyć ważność całego testu apartamenty i (b) egzekwują granice, które powinny być swobodnie zmieniać, co może obniżyć produktywność. Chciałbym, żeby więcej ludzi słuchało ich bólu i kwestionowało to przekonanie.
Ant P
3
Nie ma takiego zamieszania. Testuj jednostki zachowania, a nie jednostki kodu. Nie ma nic w koncepcji testu jednostkowego - z wyjątkiem szeroko rozpowszechnionego testowania oprogramowania w kulcie ładunków - co sugeruje, że jednostka testowa jest sprzężona z koncepcją klasy OOP. I to bardzo szkodliwe nieporozumienie.
Ant P
1
Nie jestem pewien, co dokładnie masz na myśli - argumentuję dokładnie przeciwnie do tego, co sugerujesz. Krąż / wykpiwaj twarde granice (jeśli absolutnie musisz) i przetestuj jednostkę behawioralną . Za pośrednictwem publicznego interfejsu API. To, czym jest ten interfejs API, zależy od tego, co budujesz, ale jego powierzchnia powinna być minimalna. Dodanie abstrakcji, polimorfizmu lub kodu restrukturyzacyjnego nie powinno powodować niepowodzenia testów - „jednostka na klasę” zaprzecza temu i całkowicie podważa wartość testów.
Ant P
1
RobbieDee - mówię o dokładnym przeciwieństwie. Pomyśl, że możesz być tym, który ma powszechne nieporozumienia, powróć do tego, co uważasz za „jednostkę”, a może zagłębisz się w to, o czym tak naprawdę mówił Kent Beck, kiedy mówił o TDD. To, co większość ludzi nazywa obecnie TDD, to potworność kultowa. youtube.com/watch?v=EZ05e7EMOLM
Ant P
1

Patrząc na twój projekt, sugerowałbym, że a Carskłada się z zestawu Features. Cechą być może CarRadio, lub EntertainmentSystemktóry może składać się z rzeczy takie jak CarRadio, DvdPlayeretc.

Co najmniej musiałbyś zbudować Carzestaw z Features. EntertainmentSystemi CarRadiobyłyby implementatorami interfejsu (Java, C # itp.) public [I|i]nterface Featurei byłoby to wystąpienie wzorca projektowania złożonego.

Dlatego zawsze konstruowałbyś Carz, Featuresa test jednostkowy nigdy nie utworzyłby Carbez Features. Chociaż możesz rozważyć wyśmiewanie go, BasicTestCarFeatureSetktóry ma minimalny zestaw funkcji wymaganych do najbardziej podstawowego testu.

Tylko kilka myśli.

Jan

johnd27
źródło