Nauka TDD. Prosty przewodnik
Jeśli przeczytałeś mój poprzedni wpis o TDD: Czym jest TDD i czy warto je stosować?, to z pewnością wiesz już, że warto. Problem jednak w tym, jak? Jak nauczyć się stosować TDD w codziennej pracy? W tym wpisie chciałbym się podzielić moim sposobem na praktykowanie Test Driven Development. Wypracowałem go przez lata nauki i naprawdę wielu podejść do opanowania tej metodologii. Mam nadzieję, że przyda się Tobie w nauce i przełoży się na wyższą jakość Twoich testów oraz satysfakcję z wykonywanej pracy :)
Problem pustej kartki
Nie wiem, jak jest u Ciebie, ale mi najwięcej czasu zajmuje napisanie pierwszego testu do nowo pisanej funkcjonalności. W mojej ocenie to przynajmniej 30% całego czasu spędzonego nad unit testami. To właśnie w tym miejscu wielu programistów/programistek się poddaje i odpuszcza naukę TDD, a potencjał tej metodologii w pełni pokazuje się dopiero później. Jak zatem pomóc sobie w tym wymagającym etapie?
Spróbujmy przeprowadzić sobie takie “case studies” na przykładzie kodu z mojej pierwszej gry Galactic War. Tak btw. to polecam sprawdzić samą grę :)
Na warsztat weźmiemy klasę ControlPlane. Ta klasa odpowiada za przechwytywanie i reagowanie na input od gracza. Jak cały projekt, tak i ta klasa oparte są na frameworku Qt5. Gdy poprosiłem znajomych o przetestowanie gry, zauważyłem problem uciekającej myszki z ekranu podczas rozgrywki. Domyślnie gra wchodziła w pauzę, gdy kursor wychodził poza, niewielkiej wielkości, okno gry, co frustrowało moich testerów. Czas najwyższy (ha! od 2021 roku nie dotykałem tego projektu) zająć się tym problemem. Postanowiłem, że podczas gry, przy obsłudze sygnału przesunięcia myszy, czyli slot mouseMoveEvent, będę sprawdzał, czy kursor nie zbliżył się zbyt mocno do krawędzi okna gry i jeżeli tak, to cofnę jego pozycję. Wiem, że istnieją pewnie lepsze rozwiązania, ale na ten moment wydaje mi się ono wystarczające. Zatem zaczynamy!
Żeby było zabawniej, to wprawdzie mam unit testy dla klasy ControlPlane, ale już dla tej konkretnej metody - nie :) Cóż, mogę jedynie podziękować sobie z przeszłości za to niedopatrzenie i zabrać się do pracy. A więc co z tym syndromem pustej kartki - w tym przypadku zamiast kartki mamy raczej ekran :D Początki bywają najtrudniejsze, również w praktykowaniu TDD. Wiemy, jakie jest nowe wymaganie, tylko jak je przetestować? Który scenariusz wybrać?
Zawsze wybieraj najprostszy i jak najszybciej zacznij coś pisać! Niby banał, ale serio pomaga, napisz pusty test z nazwą równie prostą:
TEST_F(ControlPlaneTests, test)
{
EXPECT_EQ(0, 0);
}
Widzisz, już ekran nie jest pusty i mamy się na czym skupić. Najpierw nazwa, wykorzystajmy tutaj standard AAA - pisałem o nim w tym wpisie. Polecam zapoznać się z nim, jeżeli go nie kojarzysz!
Nazwa testu to fundament
Nazwa powinna spełniać założenia triple A. Wiemy, że chcemy dopisać test do metody mouseMoveEvent oraz to, że ma być to najprostszy przypadek. Jaki to może być przypadek? Ano taki, który nic nie robi. Z założenia pozycja kursora powinna się zmienić, jeżeli będzie za blisko krawędzi okna gry. Nasz pierwszy test będzie zatem przypadkiem, gdy kursor myszy jest w samym środku okna i nic metoda nie powinna zmieniać.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorInTheCenter_ShouldNotChangeCursorPosition)
{
EXPECT_EQ(0, 0);
}
Teraz już wiemy, co trzeba zrobić w naszym teście.
Arrange testu
Do naszego unit testu dodajmy teraz obiekt testowanej klasy. Nie dodawaj od razu wszystkiego. Małymi krokami, tylko obiekt i niezbędne elementy do jego stworzenia.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorInTheCenter_ShouldNotChangeCursorPosition)
{
QWidget displayWidget{};
ControlPlane controller{ &displayWidget };
EXPECT_EQ(0, 0);
}
W moim przypadku do prawidłowego utworzenia testowanego obiektu potrzebowałem wskaźnika na QWidget. Możliwe, że nasuwa Ci się pytanie, dlaczego nie użyłem tutaj mocka lub stuba? Racja, nie użyłem, nie odciąłem tej zależności. Nie zrobiłem tego, ponieważ nie było mi to do niczego potrzebne. To, czy użyłbym obiektu produkcyjnego czy fałszywego, w tym przypadku nie ma znaczenia, bo nie wpływa on na przebieg testu.
Będziesz natrafiać na przypadki, gdzie sekcja Arrange będzie zawierała tylko konstruktor, czasem nie będzie jej wcale, innym razem będzie ona największą z sekcji testu i to jest ok.
Jeżeli przekazywany parametr jest wskaźnikiem, a obiekt testowanej klasy może być utworzony, gdy wartość parametru jest nullptr, to właśnie taką podaj. U mnie nie było to możliwe w obecnej formie, w jakiej zastałem mój projekt :)
W kodzie legacy właśnie ta sekcja unit testu bywa najbardziej wymagająca i nierzadko trzeba zrefaktoryzować część produkcyjnego kodu, by móc na przykład zastosować Dependency Injection. Jak to zrobić na przykładzie odcinania zależności globalnych, to odsyłam do mojego wpisu poświęconego temu zagadnieniu.
Act testu
Następną rzeczą, jaką dodałem do mojego testu, jest wywołanie testowanej metody (sekcja Act). Tutaj miałem ułatwione zadanie, ponieważ korzystam z gotowego API z frameworka Qt, więc dokładnie wiedziałem, jakich parametrów użyć. Jeśli tworzysz całkowicie nową metodę od zera, na tym etapie nie dodawałbym do niej jeszcze żadnych parametrów. Zostaw to do momentu, gdy zmusi Cię do tego kolejny krok. Już na tym etapie mogą pojawić się pierwsze pokusy refaktoryzacji. Na przykład: mógłbym stworzyć metodę pomocniczą do szybkiej kreacji obiektu QMouseEvent. Świadomie tego jednak nie robię – jest na to za wcześnie. Na sprzątanie i upiększanie kodu przyjdzie jeszcze czas ;)
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorInTheCenter_ShouldNotChangeCursorPosition)
{
QWidget displayWidget{};
ControlPlane controller{ &displayWidget };
QMouseEvent mouseEvent{
QEvent::MouseMove,
QPointF{ def::halfSceneWidth, def::halfSceneHeight },
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
};
controller.mouseMoveEvent(&mouseEvent);
EXPECT_EQ(0, 0);
}
Sekcja Act najczęściej obejmuje jedno, konkretne wywołanie testowanej metody. Dokładnie tak jak w tym przypadku. “Często” nie oznacza jednak “zawsze”. Jeśli do osiągnięcia konkretnego stanu testowanego obiektu musisz wywołać daną metodę kilka razy (bo np. testujesz maszynę stanów lub sekwencję zdarzeń), to te kilka wywołań również powinno znaleźć się w sekcji Act. Dobrą praktyką jest wtedy dopisanie odpowiedniej wzmianki o tym w samej nazwie testu.
Uważaj jednak, by nie wpleść w tej sekcji kilku wywołań, które tak naprawdę powinny być osobnymi testami. Sekcja Act musi obejmować konkretny, jeden przypadek testowy.
Asercje - tutaj dzieje się najwięcej
Nadszedł czas na ostatni element naszego unit testu. To w sekcji Assert mamy najwięcej roboty (no chyba, że jesteśmy już po ostrym refaktorze w Arrange :) ). To w ostatniej sekcji testu zderzamy się z tym, co wiemy, a czego jeszcze nie wiemy i musimy się dowiedzieć. Często bywa tak, że nie wiesz tak dokładnie, jakie są oczekiwania. Gdy piszemy kod w standardowy sposób, te oczekiwania są rozmyte. Nierzadko dopowiadasz, co powinno się wydarzyć w jednym warunku, a co w innym. W tym podejściu pojawiają się rysy na naszym kodzie i projekcie w postaci łamania fundamentalnych zasad programowania, jak KISS czy YAGNI. TDD chroni nas, byśmy nie popłynęli za mocno w naszym flow i skupili się na konkretnych krokach i etapach rozwoju. Tylko tyle, by “zazielenić” test ;)
W przypadku mojego testu asercja musi obejmować weryfikowanie wewnętrznego zapisu pozycji kursora myszy. Obecnie nasza testowana klasa w uproszczeniu wygląda tak:
class ControlPlane : public QGraphicsView
{
Q_OBJECT
public:
ControlPlane(QWidget* displayWidget);
virtual ~ControlPlane();
signals:
//...
public slots:
//...
void mouseMoveEvent(QMouseEvent* event);
//...
private:
//...
QPointF m_movePosition;
//...
};
Aby dostać się do pola m_movePosition stosuję najmniej inwazyjną metodę przeniesienia potrzebnego pola do sekcji protected, a następnie w pliku źródłowym unit testów definiuję klasę pochodną Testable z potrzebną metodą dostępową.
class ControlPlaneTestable : public ControlPlane
{
public:
using ControlPlane::ControlPlane;
auto getMovePosition() const -> QPointF
{
return m_movePosition;
}
};
Następnie wystarczy użyć tak przygotowaną klasę i zweryfikować naszą testowaną metodę.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorInTheCenter_ShouldNotChangeCursorPosition)
{
const QPointF expectedMousePos{ def::halfSceneWidth, def::halfSceneHeight };
QMouseEvent mouseEvent{
QEvent::MouseMove,
expectedMousePos,
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
};
QWidget displayWidget{};
ControlPlaneTestable controller{ &displayWidget };
controller.mouseMoveEvent(&mouseEvent);
EXPECT_EQ(expectedMousePos, controller.getMovePosition());
}
Tutaj ważna uwaga: klasa typu Testable powinna być zdefiniowana w pliku źródłowym unit testów, nie powinna być dostępna na zewnątrz. Jej celem jest tylko umożliwienie dostępu do weryfikacji wewnętrznego stanu jej klasy bazowej.
Pierwsza implementacja
Skoro nasz pierwszy unit test jest gotowy, czas przejść do implementacji. Jeśli nowy test obejmuje testowanie jeszcze nieistniejącej metody to naszym wynikiem RED będzie błąd kompilacji i to jest ok. Czas zazielenić test! Ja swój uruchomiłem i okazało się, że przechodzi. Czasem tak się dzieje, ale powinna zapalić Ci się czerwona lampka. Czy aby na pewno test sprawdza to, co powinien?
W moim przypadku implementacja już istnieje, a moim celem jest jej rozszerzenie.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
if(controller_state::activated == m_state and
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Pierwsze co rzuciło mi się w oczy to fakt, iż by zapisać pozycję kursora myszy, obiekt klasy ControlPlane musi zostać aktywowany. Szybkie sprawdzenie konstruktora rozwiązało zagadkę.
ControlPlane::ControlPlane(QWidget* displayWidget)
: QGraphicsView(displayWidget),
//...
m_state(controller_state::deactivated),
//...
m_movePosition(def::halfSceneWidth, def::halfSceneHeight),
//...
{
//...
m_controlMode = control_mode::mouse;
//...
deactivate();
}
m_state rzeczywiście było w stanie deactivated, ale domyślna pozycja myszy jest ustawiona właśnie w środku okna gry. Dlatego test od razu zaświecił się na zielono. Poprawiłem pozycję na nieco przesuniętą od środka i test stał się czerwony.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition)
{
const QPointF expectedMousePos{ def::halfSceneWidth + 2, def::halfSceneHeight - 1 };
QMouseEvent event{
QEvent::MouseMove,
expectedMousePos,
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
};
QWidget displayWidget{};
ControlPlaneTestable controller{ &displayWidget };
controller.mouseMoveEvent(&event);
EXPECT_THAT(expectedMousePos, Eq(controller.getMovePosition()));
}
Zmianie uległa również nazwa testu. Oto wynik:
[ RUN ] ControlPlaneTests.mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition
Expected equality of these values:
expectedMousePos
Which is: 16-byte object <00-00 00-00 00-E0 72-40 00-00 00-00 00-F0 78-40>
controller.getMovePosition()
Which is: 16-byte object <00-00 00-00 00-C0 72-40 00-00 00-00 00-00 79-40>
[ FAILED ] ControlPlaneTests.mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition (8 ms)
Teraz mam pewność, że sprawdza to co powinien. Jeszcze szybka poprawa logowania, by dało się je odczytać po ludzku. Więcej o tym jak, zwiększyć czytelność wyników testu znajdziesz tutaj.
[ RUN ] ControlPlaneTests.mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition
Value of: controller.getMovePosition()
Expected: is equal to x = 302, y = 399
Actual: x = 300, y = 400 (of type QPointF)
[ FAILED ] ControlPlaneTests.mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition (8 ms)
Następnie aktywujemy nasz ControlPlane poprzez dodanie wywołania metody activate i test znów jest zielony. Tym razem poprawnie zielony.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition)
{
const QPointF expectedMousePos{ def::halfSceneWidth + 2, def::halfSceneHeight - 1 };
QMouseEvent event{
QEvent::MouseMove,
expectedMousePos,
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
};
QWidget displayWidget{};
ControlPlaneTestable controller{ &displayWidget };
controller.setState(controller_state::activated);
controller.mouseMoveEvent(&event);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Pierwsza iteracja a refactoring
Kiedy nasz test jest już zielony, według TDD następnym elementem cyklu jest Refactoring. W pierwszej iteracji odradzałbym jednak od razu go robić. Zbyt mało wiemy, zarówno o tym jak powinna wyglądać implementacja, jak i testy. Mimo to warto się na chwilę zatrzymać i przemyśleć czy możemy coś delikatnie poprawić. Zwróć uwagę czy zastosowane w teście zmienne mogą być stałymi, czy są poprawnie zainicjalizowane, czy konwencje projektowe są zachowane i tym podobne sprawy. Pełny refaktor zalecam zrobić dopiero po drugim lub nawet trzecim teście i regularnie powtarzać go już w każdej iteracji.
Drugi test
Następny unit test powinien być tylko jednym krokiem naprzód. Można zastanowić się, jaki jeszcze scenariusz może prowadzić do tego samego rezultatu co w pierwszym teście. Jedyna różnica będzie wtedy w sekcji Arrange. Szukamy przede wszystkim scenariusza brzegowego. W moim przypadku przyjmując założenie, że offset od granicy okna gry wynosi 15 pikseli, muszę sprawdzić, czy gdy kursor będzie na granicy tej wartości, to dalej pozostanie w swojej pozycji.
TEST_F(ControlPlaneTests, mouseMoveEvent_Cursor15PxFromTopEdgeAndHorizontallyCentered_ShouldNotChangeCursorPosition)
{
const QPointF expectedMousePos{ def::halfSceneWidth, 15 };
QMouseEvent event{
QEvent::MouseMove,
expectedMousePos,
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
};
QWidget displayWidget{};
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&event);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Takich testów, gdzie zachowanie jest takie samo jak w poprzednim teście, może być więcej. Wszystko zależy od wymagań.
Pierwszy test weryfikujący nowe zachowanie
Czas na unit test, który będzie weryfikować nowe zachowanie spełniające nasze wymaganie, by kursor myszy cofał się do wyznaczonego odsunięcia od krańca okna gry, gdy przekroczy pewien próg. Naszym offsetem jest 15 pikseli, więc test musi sprawdzić, czy pozycja myszy się zmieni. Poniżej nowy test.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge)
{
const QPointF expectedMousePos{ def::halfSceneWidth, 15 };
const QPointF topMousePos{ def::halfSceneWidth, 14 };
QMouseEvent event{
QEvent::MouseMove,
topMousePos,
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
};
QWidget displayWidget{};
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&event);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Po uruchomieniu nasz nowy test jest czerwony.
[ RUN ] ControlPlaneTests.mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge
Value of: controller.getMovePosition()
Expected: is equal to x = 300, y = 15
Actual: x = 300, y = 14 (of type QPointF)
[ FAILED ] ControlPlaneTests.mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge (9 ms)
Mamy RED, teraz GREEN, więc przechodzimy do implementacji. Dodajemy tylko tyle kodu, by spełnić warunki testu. Dodaję tylko jeden warunek, sprawdzam, czy pozycja myszy jest mniejsza od offsetu.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minYPos{ 15.0F };
if(controller_state::activated == m_state and
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Na tym etapie mógłbym dodać pozostałe warunki, ale jeszcze tego nie zrobię. Po ponownym odpaleniu testów, wszystkie są zielone. Czas na refaktoryzację. Sama implementacja w mojej ocenie, w tym momencie jej nie potrzebuje, jednak testy - tak!
Refaktoryzacja
Podczas pracy w TDD z pewnością dojdziesz do momentu, gdy zauważysz powtórzenia w testach. Zazwyczaj trzeci test to już dobry moment na taki refaktor. Co możemy poprawić? Myślę, że najlepiej szukać miejsc, gdzie zysk będzie największy. Tworzenie QMouseEvent możemy przenieść do metody narzędziowej. Dzięki temu wszystkie nasze testy mocno się skrócą.
class ControlPlaneTests : public Test
{
public:
//...
static auto getMouseEvent(const QPointF& position) -> QMouseEvent
{
return QMouseEvent(
QEvent::MouseMove,
position,
Qt::NoButton,
Qt::NoButton,
Qt::NoModifier
);
};
};
//...
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge)
{
const QPointF expectedMousePos{ def::halfSceneWidth, 15 };
const QPointF topMousePos{ def::halfSceneWidth, 14 };
QMouseEvent event{ getMouseEvent(topMousePos) };
QWidget displayWidget{};
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&event);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Uruchamiamy testy i dalej wszystkie świecą na zielono. To bardzo ważne, by przy każdej zmianie, uruchomić testy i sprawdzić, czy nie wprowadziliśmy regresji. Kolejny powtarzalny element to displayWidget. Przenieśmy go do klasy ControlPlaneTests.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge)
{
const QPointF expectedMousePos{ def::halfSceneWidth, 15 };
const QPointF topMousePos{ def::halfSceneWidth, 14 };
QMouseEvent event{ getMouseEvent(topMousePos) };
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&event);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Testy dalej zielone. Uwzględniając wszystkie unit testy, tym jednym prostym ruchem, zastąpiłem 25 linii kodu jedną :) . Ostatnia rzecz, która mi się nasuwa to topMousePos. Jak dla mnie, wystarczy lepiej nazwać event i usunąć topMousePos.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge)
{
const QPointF expectedMousePos{ def::halfSceneWidth, 15 };
QMouseEvent mouseMoveEventBelowMinY{ getMouseEvent({ def::halfSceneWidth, 14 }) };
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&mouseMoveEventBelowMinY);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Kolejny raz, szybkie uruchomienie testów i również tym razem wszystkie przechodzą. Ok, myślę, że dalsze próby refaktoryzacji już nie są potrzebne.
Kolejne iteracje
Zatoczyliśmy pełny cykl, co dalej? Iterujemy dalej, dokładając kolejne przypadki testowe. Zawsze dokładamy jeden test na raz. O podobnej złożoności lub minimalnie większej. W tym przypadku dodajemy przypadki testowe sprawdzające pozostałe granice okna gry, dodając kolejne części implementacji. Najpierw dolna krawędź.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromBottomEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromBottomEdge)
{
const QPointF expectedMousePos{ def::halfSceneWidth, def::sceneHeight - 15 };
QMouseEvent mouseMoveEventAboveMaxY{ getMouseEvent({ def::halfSceneWidth, def::sceneHeight - 14 }) };
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&mouseMoveEventAboveMaxY);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Mamy RED, teraz implementacja.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minYPos{ 15.0F };
if(controller_state::activated == m_state and
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
else if(m_movePosition.y() > def::sceneHeight - minYPos)
{
m_movePosition.setY(def::sceneHeight - minYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Mamy GREEN. Wyrażenie def::sceneHeight - minYPos pojawia się w kodzie dwa razy i ma określone znaczenie. Dodaję stałą maxYPos. Poprawiam także kolejność elementów w else if, by warunek był bardziej intuicyjny.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minYPos{ 15.0F };
static const auto maxYPos{ def::sceneHeight - minYPos };
if(controller_state::activated == m_state and
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
else if(maxYPos < m_movePosition.y())
{
m_movePosition.setY(maxYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Dalej, sprawdzamy lewą krawędź.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromLeftEdgeAndVerticallyCentered_ShouldChangeCursorPositionTo15PxFromLeftEdge)
{
const QPointF expectedMousePos{ 15, def::halfSceneHeight };
QMouseEvent mouseMoveEventLeft{ getMouseEvent({ 14, def::halfSceneHeight }) };
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&mouseMoveEventLeft);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Ponownie RED i implementacja.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minYPos{ 15.0F };
static const auto maxYPos{ def::sceneHeight - minYPos };
if(controller_state::activated == m_state and
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
else if(maxYPos < m_movePosition.y())
{
m_movePosition.setY(maxYPos);
}
else if(m_movePosition.x() < minYPos)
{
m_movePosition.setX(minYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
I GREEN. Widać, że stała minYPos dotyczy już nie tylko osi Y, ale również osi X. Poprawiam nazwę na minEdgePos. Tak samo dla maxYPos. I przechodzę do prawej krawędzi.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromRightEdgeAndVerticallyCentered_ShouldChangeCursorPositionTo15PxFromRightEdge)
{
const QPointF expectedMousePos{ def::sceneWidth - 15, def::halfSceneHeight };
QMouseEvent mouseMoveEventRight{ getMouseEvent({ def::sceneWidth - 14, def::halfSceneHeight }) };
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&mouseMoveEventRight);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
I znów, nowy RED. Dodajemy niezbędną implementację.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minEdgePos{ 15.0F };
static const auto maxEdgePos{ def::sceneHeight - 15.0F };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.y() < minEdgePos)
{
m_movePosition.setY(minEdgePos);
}
else if(maxEdgePos < m_movePosition.y())
{
m_movePosition.setY(maxEdgePos);
}
else if(m_movePosition.x() < minEdgePos)
{
m_movePosition.setX(minEdgePos);
}
else if(maxEdgePos< m_movePosition.x())
{
m_movePosition.setX(maxEdgePos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Ale test dalej świeci na czerwono. Co się stało? Ano popełniłem błąd. No cóż, to się zdarza. Problem tkwi w maxEdgePos. Stała ta bazuje na wysokości sceny. Natomiast my dla sprawdzenia osi X potrzebujemy szerokości.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minEdgePos{ 15.0F };
static const auto maxEdgePos{ def::sceneHeight - 15.0F };
static const auto maxXEdgePos{ def::sceneWidth - 15.0F };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.y() < minEdgePos)
{
m_movePosition.setY(minEdgePos);
}
else if(maxEdgePos < m_movePosition.y())
{
m_movePosition.setY(maxEdgePos);
}
else if(m_movePosition.x() < minEdgePos)
{
m_movePosition.setX(minEdgePos);
}
else if(maxXEdgePos< m_movePosition.x())
{
m_movePosition.setX(maxXEdgePos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Mamy GREEN. Czas zmienić nieco podejście. Wracamy do maxYPos, więc wrócimy również do minYPos. Dla większej czytelności. A że oś X najczęściej jest wymieniana jako pierwsza, kolejność warunków również zmieniłem.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minXPos{ 15.0F };
static const auto maxXPos{ def::sceneWidth - 15.0F };
static const auto minYPos{ 15.0F };
static const auto maxYPos{ def::sceneHeight - 15.0F };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.x() < minXPos)
{
m_movePosition.setX(minXPos);
}
else if(maxXPos< m_movePosition.x())
{
m_movePosition.setX(maxXPos);
}
else if(m_movePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
else if(maxYPos < m_movePosition.y())
{
m_movePosition.setY(maxYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Każde kolejne przejście z czerwonego do zielonego stanu bywa satysfakcjonujące. Możesz wtedy poczuć, że rzeczywiście poruszasz się naprzód niczym taran. To jest właśnie potężny psychologiczny, pozytywny aspekt stosowania TDD :)
Corner case’y
Możesz sobie pomyśleć, brawo, wszystkie granice ekranu gry ogarnięte i obsłużone. Ale czy na pewno? Zanim przejdziemy do otwierania szampana czy zamawiania pizzy zwycięstwa (czy jakiegokolwiek innego sposobu uczczenia sukcesu), zastanówmy się czy nasze testy pokrywają wszystkie przypadki, które mogą wystąpić. Mamy sprawdzanie każdej strony ekranu osobno, a co się stanie, gdy nasz kursor myszy będzie w górnym, lewym narożniku. Czy nasz kod zadziała?
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromTopAndLeftEdges_ShouldChangeCursorPositionTo15PxFromTopAndLeftEdges)
{
const QPointF expectedMousePos{ 15, 15 };
QMouseEvent mouseMoveEventTopLeft{ getMouseEvent({ 14, 14 }) };
ControlPlaneTestable controller{ &displayWidget };
controller.activate();
controller.mouseMoveEvent(&mouseMoveEventTopLeft);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
Wynik.
[ RUN ] ControlPlaneTests.mouseMoveEvent_CursorLessThan15PxFromTopAndLeftEdges_ShouldChangeCursorPositionTo15PxFromTopAndLeftEdges
Value of: controller.getMovePosition()
Expected: is equal to x = 15, y = 15
Actual: x = 15, y = 14 (of type QPointF)
[ FAILED ] ControlPlaneTests.mouseMoveEvent_CursorLessThan15PxFromTopAndLeftEdges_ShouldChangeCursorPositionTo15PxFromTopAndLeftEdges (20 ms)
A więc mamy kolejny błąd! Cóż, wracając do kodu dość szybko znalazłem, co było jego przyczyną.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minXPos{ 15.0F };
static const auto maxXPos{ def::sceneWidth - 15.0F };
static const auto minYPos{ 15.0F };
static const auto maxYPos{ def::sceneHeight - 15.0F };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.x() < minXPos)
{
m_movePosition.setX(minXPos);
}
else if(maxXPos< m_movePosition.x())
{
m_movePosition.setX(maxXPos);
}
else if(m_movePosition.y() < minYPos) // <---- z rozpędu dodałem `else`, a przecież powinniśmy sprawdzać obie osie niezależnie
{
m_movePosition.setY(minYPos);
}
else if(maxYPos < m_movePosition.y())
{
m_movePosition.setY(maxYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
I już poprawiona wersja.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minXPos{ 15.0F };
static const auto maxXPos{ def::sceneWidth - 15.0F };
static const auto minYPos{ 15.0F };
static const auto maxYPos{ def::sceneHeight - 15.0F };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
m_movePosition = event->pos();
if(m_movePosition.x() < minXPos)
{
m_movePosition.setX(minXPos);
}
else if(maxXPos < m_movePosition.x())
{
m_movePosition.setX(maxXPos);
}
if(m_movePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
else if(maxYPos < m_movePosition.y())
{
m_movePosition.setY(maxYPos);
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
I znów wszystkie nasze testy błyszczą zielenią.
[----------] Global test environment tear-down
[==========] 28 tests from 1 test suite ran. (616 ms total)
[ PASSED ] 28 tests.
Po dopisaniu testów dla pozostałych narożników tylko upewniłem się, że kod działa tak jak powinien.
Pominięte wymaganie
Doszliśmy do momentu, gdzie dopisanie nowych unit testów nic dodatkowego nie wniesie. Wszystkie kluczowe przypadki zostały pokryte. Zadowolony, postanowiłem przetestować nową funkcjonalność podczas rozgrywki i ku mojemu zdziwieniu okazało się, że kursor myszy po zbliżeniu do dowolnej krawędzi dalej mknie przed siebie. Po dodaniu logów dalej widzę, że sama logika poprawy pozycji kursora działa bez zarzutu. Problem był w pominiętym wymaganiu. Metoda powinna nie tylko korygować pozycję kursora, ale również sam kursor ustawiać w nowej pozycji, czego w moim kodzie zabrakło.
Czasem może się zdarzyć, że coś pominiemy. Nie uwzględniłem tego na początku implementacji metody. W Qt pozycję kursora możemy ustawić, między innymi, za pomocą statycznej metody QCursor::setPos(). Aby umożliwić przetestowanie wywołania tej metody, dodamy wrapper.
class CursorController
{
public:
virtual ~CursorController() = default;
virtual void setCursorPosition(const QPoint& position);
};
void CursorController::setCursorPosition(const QPoint& position)
{
QCursor::setPos(position);
}
I wstrzykniemy CursorController za pomocą nowego parametru konstruktora ControlPlane wraz z domyślną wartością.
class ControlPlane : public QGraphicsView
{
Q_OBJECT
public:
ControlPlane(
QWidget* displayWidget,
std::unique_ptr<CursorController> cursorController = std::make_unique<CursorController>()
);
virtual ~ControlPlane();
//...
Tym razem użyłem smart pointera, mały krok by poprawić projekt legacy :) Przejdźmy jeszcze do mocka i zastosujmy go w testach.
class CursorControllerMock : public CursorController
{
public:
MOCK_METHOD(void, setCursorPosition, (const QPoint& position), (override));
};
Dodałem mocka do pierwszego testu. Zakłada on, że kursor nie zmienia pozycji, więc metoda setCursorPosition naszego mocka nie powinna zostać wywołana.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorNearTheCenterOfDisplay_ShouldNotChangeCursorPosition)
{
const QPointF expectedMousePos{ def::halfSceneWidth + 2, def::halfSceneHeight - 1 };
QMouseEvent event{ getMouseEvent(expectedMousePos) };
EXPECT_CALL(*cursorControllerMock, setCursorPosition(_)).Times(0);
ControlPlaneTestable controller{ &displayWidget, std::move(cursorControllerMock) };
controller.activate();
controller.mouseMoveEvent(&event);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
W tym przypadku implementacja się nie zmienia. Przechodzę do pierwszego testu, gdzie położenie kursora powinno się zmienić.
TEST_F(ControlPlaneTests, mouseMoveEvent_CursorLessThan15PxFromTopEdgeAndHorizontallyCentered_ShouldChangeCursorPositionTo15PxFromTopEdge)
{
const QPointF expectedMousePos{ def::halfSceneWidth, 15 };
QMouseEvent mouseMoveEventBelowMinY{ getMouseEvent({ def::halfSceneWidth, 14 }) };
EXPECT_CALL(*cursorControllerMock, setCursorPosition(_)).Times(1);
ControlPlaneTestable controller{ &displayWidget, std::move(cursorControllerMock) };
controller.activate();
controller.mouseMoveEvent(&mouseMoveEventBelowMinY);
EXPECT_THAT(controller.getMovePosition(), Eq(expectedMousePos));
}
By spełnić kryteria obu testów, musimy zweryfikować czy pozycja kursora odczytana z event’u została skorygowana czy nie i jeśli tak, to tylko w tym przypadku pozycja kursora powinna być zaktualizowana. Nowa implementacja wygląda następująco.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const auto minXPos{ 15.0F };
static const auto maxXPos{ def::sceneWidth - 15.0F };
static const auto minYPos{ 15.0F };
static const auto maxYPos{ def::sceneHeight - 15.0F };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
const auto rawMovePosition{ event->pos() };
m_movePosition = rawMovePosition;
if(rawMovePosition.x() < minXPos)
{
m_movePosition.setX(minXPos);
}
else if(maxXPos < rawMovePosition.x())
{
m_movePosition.setX(maxXPos);
}
if(rawMovePosition.y() < minYPos)
{
m_movePosition.setY(minYPos);
}
else if(maxYPos < rawMovePosition.y())
{
m_movePosition.setY(maxYPos);
}
if(rawMovePosition != m_movePosition)
{
m_cursorController->setCursorPosition(m_displayWidget->mapToGlobal(m_movePosition.toPoint()));
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Testy są zielone. Zaktualizowałem również pozostałe. Teraz mamy spełniony komplet wymagań i możemy cieszyć się lepszą jakością rozgrywki. Wciąż istnieje możliwość wyjścia kursorem myszy poza ekran i przejścia do pauzy gry, jednak jest to mocno ograniczone i mniej wprawieni gracze z pewnością odczują różnicę :)
Plastyczność kodu
Gdy już każdy test został napisany, a każde wymaganie spełnione, dochodzimy do mojego ulubionego etapu pracy z kodem. W tym momencie każda zmiana, która łamie nasze założenia, zostanie od razu wykryta, więc możemy poeksperymentować z innymi wersjami implementacji. W moim odczuciu kod staje się “plastyczny”, można go dowolnie formować i zmieniać, bo jeśli coś pójdzie nie tak, każda pomyłka od razu zostanie wyłapana przez nasze unit testy, a my możemy się z nietrafionego pomysłu wycofać i spróbować czegoś innego.
W pierwszym kroku polecam obecną wersję zcommitować w naszym lokalnym repozytorium i jeśli wymyślimy lepszą wersję, po prostu nadpisać commit(np. git commit --amend).
A oto inna wersja implementacji z użyciem std::min i std::max.
void ControlPlane::mouseMoveEvent(QMouseEvent* event)
{
static const double minXPos{ 15.0 };
static const double maxXPos{ def::sceneWidth - 15.0 };
static const double minYPos{ 15.0 };
static const double maxYPos{ def::sceneHeight - 15.0 };
if(controller_state::activated == m_state &&
control_mode::mouse == m_controlMode)
{
const QPointF rawMovePosition{ event->pos() };
m_movePosition.setX(std::max(minXPos, std::min(rawMovePosition.x(), maxXPos)));
m_movePosition.setY(std::max(minYPos, std::min(rawMovePosition.y(), maxYPos)));
if(rawMovePosition != m_movePosition)
{
QCursor::setPos(m_displayWidget->mapToGlobal(m_movePosition.toPoint()));
}
emit changeMovePosition(m_movePosition);
}
event->ignore();
}
Ta wersja jest zdecydowanie krótsza, bardziej zwięzła. Tylko czy zawsze mniej linii kodu to lepiej? Po dokładnym przyjrzeniu się, widzę dwie wady tego rozwiązania. Po pierwsze, kod jest mniej czytelny. Trzeba chwilę dłużej się zastanowić, co i kiedy jest porównywane. W pierwszej wersji, warunki są proste i dobrze widoczne, a kod pod nimi jasny i zrozumiały. Druga wada to minimalnie mniejsza wydajność, ponieważ aby ustawić wartość pojedynczej osi, za każdym razem sprawdzamy dwa warunki. Nie jest to pewnie kolosalna różnica, ale jednak zawsze coś :)
Pozostaję przy pierwszej implementacji i kończę pracę nad tą funkcjonalnością.
Podsumowanie
Nasze case study dobiegło końca. To do tej pory, mój najdłuższy wpis na blogu i zajął mi najwięcej czasu. Dotychczas starałem się publikować jeden post na miesiąc. Praca nad tym konkretnym zajęła mi prawie 4 miesiące - o rany! To nie jest tak, że dłubałem przy nim bite miesiące. Dużo innych spraw oraz rozległość samego wpisu wpłynęły na czas jego tworzenia. Niemniej jednak cieszę się, że udało mi się go zrealizować :) Mam nadzieję, że treść, którą się tutaj z Tobą podzieliłem, pomoże Ci w codziennej pracy i rozwoju Twojej kariery. Będzie mi bardzo miło, jeśli podzielisz się swoją opinią w komentarzu lub wiadomości prywatnej :)
A tutaj podaję link do zaktualizowanej wersji mojej małej gry - GalacticWar.
Autor: Tadeusz Biela
Programista C++ | Entuzjasta TDD | Fan unit testów
Zostaw komentarz