Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: phpunit testowanie metod
Forum PHP.pl > Forum > PHP
szubi95
Cześć!

W zasadzie tak jak w temacie. Czy powinno się testować metody prywatne i chronione? Moim zdaniem raczej nie, skoro wykonują one operacje wewnętrzne klasy. Z tego co rozumiem to nas interesują dane wejściowe oraz wyjściowe, nie konkretne implementacje poszczególnych składników klasy. Jeśli się mylę, proszę o poprawienie wink.gif Z drugiej strony przy generowaniu raportu xdebug'iem mam elementy czerwone jak tego nie zrobię tongue.gif Czy powinienem faktycznie testować takie metody z użyciem Reflection API, czy po prostu sprawdzać tylko w reporcie metody publiczne?


Pozdrawiam,
szubi
Pyton_000
A co jeśli metody prywatne wykonują jakieś obliczenie, i chcesz sprawdzić czy na pewno jest ok?

Ogólnie to metod prywatnych i chronionych jak najmniej.
Crozin
Cytat
A co jeśli metody prywatne wykonują jakieś obliczenie, i chcesz sprawdzić czy na pewno jest ok?
No to sprawdzi je publicznym interfejsem obiektu? Pośrednio.
Cytat
Ogólnie to metod prywatnych i chronionych jak najmniej.
Czymkolwiek możesz to poprzeć? Bo jest to dosyć sprzeczne z zasadami OOP.

---

Cytat
Czy powinno się testować metody prywatne i chronione?
Generalnie: nie. Metody prywatne są już stricte sprawą danej klasy i mogą się właściwie w dowolnym momencie dowolnie zmieniać i nie powinno to mieć wpływu na nic poza tą klasą. Metody prywatne stanowią już pewien interfejs klasy/obiektu (w końcu jest on dostępny z zewnątrz) i o testy tego możesz się jeszcze pokusić przy czym raczej nie z wykorzystaniem ReflectionAPI, a konkretnych implementacji stworzonych specjalnie dla potrzeb testów.
Cytat
Z drugiej strony przy generowaniu raportu xdebug'iem mam elementy czerwone jak tego nie zrobię
Domyślam się, że chodzi o raporty pokrycia kodu testami? W takim razie powinieneś przygotować takie przypadki (dane wejściowe) które spowodują ich wywołanie. Nie mniej jednak nie powinieneś pisać testów dla podniecania się wielkością zielonych pasków. :-P
szubi95
Dziękuję Crozin smile.gif

Mógłbyś mi powiedzieć jeszcze, co mam rozumieć poprzez sprawdzenie ich publicznym interfejsem obiektu?

Cytat
W takim razie powinieneś przygotować takie przypadki (dane wejściowe) które spowodują ich wywołanie.


Tak chodzi o raporty pokrycia kodu. Czyli testować podstawowe rzeczy, aby test zwyczajnie przeszedł? Chyba nie rozumiem.

Cytat
Nie mniej jednak nie powinieneś pisać testów dla podniecania się wielkością zielonych pasków. :-P


Już któryś raz to widzę tongue.gif
Crozin
Metody prywatne są zapewne wywoływane pośrednio przy wykorzystaniu publicznego interfejsu obiektu za pośrednictwem jego publicznych metod. Przykładowo:
  1. class MyClass {
  2. public function doSth($sth) {
  3. // do sth
  4.  
  5. if ($sth == 123) {
  6. $this->doSthElse();
  7. }
  8.  
  9. // do sth
  10. }
  11.  
  12. private function doSthElse() {
  13. // do sth else
  14. }
  15. }
W takim przypadku powinieneś mieć co najwyżej testy metody doSth(), ale powinieneś też przygotować test tej metody z parametrem "123", który spowoduje wykonanie metody prywatnej.
com
Cytat
Ogólnie to metod prywatnych i chronionych jak najmniej.


Nie wierze w to co widzę biggrin.gif
szubi95
Rozumiem i jeszcze raz bardzo dziękuję Crozin wink.gif
Xelah
Pozwolę sobie dodać jeszcze kilka rzeczy od siebie, bo nie do końca zgodze się z Crozin-em, że nie należy się przejmowac code-coverage.

Powinieneś się tym przejmować. Z bardzo prostego powodu. Jeśli nie potrafisz przetestować klasy w ten sposób, że każda jej prywatna i chroniona metoda zostanie wywołana i wykonasz każdą linijkę kodu, to znaczy tylko jedno - kod jest martwy i możesz go usunąć bez szkody dla funkcjonalności.
Opcja druga jest taka, że masz problem z architekturą kodu. Jeśli jesteś pewny, że dany fragment da się jakoś wykonać, ale jest to niezmierne skomplikowane, to należy przemyślec architekturę tego fragmentu.
Generalnie powinieneś dążyć do 100% code-coverage. To w sumie jest banalne i nie dasz rady tego osiągnąć tylko w bardzo specyficznych przypadkach i tylko na takiej czy innej maszynie, która robi build i odpala testy.

Za to w 100% zgodzę się z przedmówcą, że testowac powinieneś tylko metody publiczne, bo to one stanowią interfejs API. Reszta to detal implmentacji i testów w ogóle nie powinno obchodzić co się tam dzieje. Ważne, żeby pbliczne API działało, bo tylko do niego masz dostep.

Jedyna sytuacja, w której możesz pokusić się o testowanie protected czy private to przypadki klas abstrakcyjnych. W takim przypadku masz do wyboru kilka opcji:
1. Testujesz bezpośrednio na klasie z użyciem Closure
2. Robisz testową klasę, która rozszerza abstrakcję i tam testujesz czy wszystko się zachowuje jak należy.
3. Robisz to bezpośrednio w testach wszystkich klas rozszeżających abstrakcję.

Trzecia opcja jest najmniej efektywna jeśli masz sporo klas dziedziczących. Sporo duplikacji tych samych testów.

Możesz też nie pokrywać testami tej klasy w ogóle, wychodząc z założenia, że jeśli klasa dziedzicząca działa, to jest OK. Ale to raczej jest ryzykowne i może spokojnie prowadzić do problemów, ale zastawiania martwego kodu, który nie jest używany.

No jeszcze muszę się nie zgodzić z Pyton_000-em. Metod prywatnych i chronionych jak najwięcej. Publiczne tylko wtedy, kiedy je potrzebujesz. Jeśli nie masz use-case-a, który uzasadnie istnienie metody publicznej to jej nie piszesz.
com
Sorry kolego, ale Pyton_000 mógł mieć gorszy dzień to mu się coś pomyliło.
A cytat jego wypowiedzi niczego nie udowadnia i widać, że nie znasz za dobrze php jak działa co zostało tam udowodnione wiec zejdź na ziemie smile.gif
com
nic nikomu nie liże, jakbyś nie zważył sam się do tego odniosłem co napisał w tym wątku, ale w tamtym nie ja się mylę tylko Ty, co znów udowodniłem wink.gif
vokiel
Jeśli mówimy o testach jednostkowych to IMHO prywatne metody powinny być jak najbardziej testowane. W przypadku testów integracyjnych, systemowych jak najbardziej wystarczy testować tylko publiczne API.

Jeśli tworzy się kod zgodnie z zasadą hermetyzacji, gdzie publiczne metody są tylko te, które muszą nimi być - to duża część logiki zawiera się w prywatnych. Je trzeba też przetestować.
Testowanie metod prywatnych poprzez publiczne przeczy zasadzie testów jednostkowych, które w założeniu mają testować najmniejsze jednostki kodu, a nie łańcuchy wywołań. Czasem przetestowanie prywatnych metod wywoływanych przez publiczne powoduje, że pisanie testów jest dużo szybsze, że trzeba ich mniej i trudniej jest coś przeoczyć.

Cytat(Xelah @ 25.06.2015, 08:16:36 ) *
Powinieneś się tym przejmować. Z bardzo prostego powodu. Jeśli nie potrafisz przetestować klasy w ten sposób, że każda jej prywatna i chroniona metoda zostanie wywołana i wykonasz każdą linijkę kodu, to znaczy tylko jedno - kod jest martwy i możesz go usunąć bez szkody dla funkcjonalności.

Za to w 100% zgodzę się z przedmówcą, że testowac powinieneś tylko metody publiczne, bo to one stanowią interfejs API. Reszta to detal implmentacji i testów w ogóle nie powinno obchodzić co się tam dzieje. Ważne, żeby pbliczne API działało, bo tylko do niego masz dostep.

I właśnie w tych detalach może skrywać się najwięcej błędów ohmy.gif

Testowanie tylko metod publicznych, które są zależne od wielu prywatnych lub od łańcucha ich wywołań powoduje konieczność wygenerowania testów dla wszystkich możliwych kombinacji, co powoduje, że jest ich dużo więcej niż, gdy testuje się metody prywatne oddzielnie.

Cytat(Crozin @ 24.06.2015, 22:14:14 ) *
Metody prywatne są zapewne wywoływane pośrednio przy wykorzystaniu publicznego interfejsu obiektu za pośrednictwem jego publicznych metod. Przykładowo:
  1. class MyClass {
  2. public function doSth($sth) {
  3. // do sth
  4.  
  5. if ($sth == 123) {
  6. $this->doSthElse();
  7. }
  8.  
  9. // do sth
  10. }
  11.  
  12. private function doSthElse() {
  13. // do sth else
  14. }
  15. }
W takim przypadku powinieneś mieć co najwyżej testy metody doSth(), ale powinieneś też przygotować test tej metody z parametrem "123", który spowoduje wykonanie metody prywatnej.


Lekko zmodyfikowany przykład:
  1. class MyClass {
  2. public function doSth($sth) {
  3. $privateResult = $this->doSthElse($sth);
  4.  
  5. if ($privateResult<0) {
  6. return $this->doSthElseLower($privateResult);
  7. } elseif ($privateResult>0) {
  8. return $this->doSthElseUpper($privateResult);
  9. } else {
  10. return $this->doSthElseZero($privateResult);
  11. }
  12. }
  13.  
  14. private function doSthElse($privateResult) {}
  15.  
  16. private function doSthElseLower($privateResult) {}
  17.  
  18. private function doSthElseUpper($privateResult) {}
  19.  
  20. private function doSthElseZero($privateResult) {}
  21. }


Testowanie tylko doSth może dawać błędne wyniki w zależności od błędów w prywatnych metodach. Żeby przetestować wszystkie możliwe scenariusze należy przetestować wszystkie kombinacje wystąpień zmiennych, które mają wpływ na wynik. Możemy nawet otrzymać poprawne wyniki w przypadku kolizji błędów w kilku metodach prywatnych.

Testowanie prywatnych powoduje, że testy będą czytelniejsze i lepiej zorganizowane, łatwiej jest też obsłużyć wszystkie scenariusze.


Ostania rzecz odnośnie samego code coverage, na które też trzeba uważać żeby nie pisać testów pod nie. Jest pokusa aby testy przeszły przez wszystkie gałęzie kodu i było 100% green, ale nie w tym rzecz. Bo same puste przejścia wcale nie gwarantują przetestowania ich efektów. To, że kod się wywołał nie oznacza, że wynik tego wywołania został sprawdzony. Już widziałem testy, gdzie Config->get() było wywoływane dziesiątki/setki razy, dobry CRAP ale co z tego, jak sama klasa Config nie była przetestowana jednostkowo, indywidualnie i nie zostały sprawdzone żadne przypadki brzegowe. Wystarczyło w configu w jednym polu wpisać -1 i fail. Setki wywołań pośrednich na niewiele się zdały.
Tuminure
Cytat
I właśnie w tych detalach może skrywać się najwięcej błędów

Błędów, które są nieistotne dla działania klasy.

Cytat
Testowanie tylko metod publicznych, które są zależne od wielu prywatnych lub od łańcucha ich wywołań powoduje konieczność wygenerowania testów dla wszystkich możliwych kombinacji, co powoduje, że jest ich dużo więcej niż, gdy testuje się metody prywatne oddzielnie.

Nawet po przetestowaniu każdej metody prywatnej oddzielnie, powinieneś przetestować wspomniane wszystkie kombinacje dla metody publicznej.

Cytat
Możemy nawet otrzymać poprawne wyniki w przypadku kolizji błędów w kilku metodach prywatnych.

Oznacza to jedynie tyle, że test jest źle napisany (lub jeżeli kolizje błędów zwracają ZAWSZE poprawne wyniki, to test jest napisany dobrze, a wspomniane błędy są nieistotne dla działania).

I nie mówię, że testowanie metod prywatnych/chronionych jest bez sensu. Po prostu zazwyczaj nie jest to ani przydatne, ani potrzebne.
Xelah
Cytat(vokiel @ 26.06.2015, 11:13:00 ) *
I właśnie w tych detalach może skrywać się najwięcej błędów ohmy.gif


I to dalej jest nieistotne z punktu widzenia API. Jeśli masz błąd w prywatnej metodzie a API dla dowolnego wejścia produkuje poprawne wyjście, to znaczo to, tyle, że masz błąd w teście. A nie w kodzie.

Cytat(vokiel @ 26.06.2015, 11:13:00 ) *
Testowanie tylko metod publicznych, które są zależne od wielu prywatnych lub od łańcucha ich wywołań powoduje konieczność wygenerowania testów dla wszystkich możliwych kombinacji, co powoduje, że jest ich dużo więcej niż, gdy testuje się metody prywatne oddzielnie.


A co Ci mówi fakt, że metoda prywatna zwróciła to, co trzeba? Z punktu widzenia API? Nic. Dosłanie nic. Na pewno nie mówi Ci, że API działa. API i tak będzie przechodziło przez całą ścieżkę i ważne jest, żeby to działało.
API testujesz tak, żeby osiągnąć 100% code-coverage. Wtedy wiesz, że sprawdziłeś każdą ścieżkę (oczywiście nie gwarantuje Ci to braku błędów). Powinieneś też sprawdzić warunki brzegowe, ale to dalej nie ma związku z metodami prywatnymi.

Cytat(vokiel @ 26.06.2015, 11:13:00 ) *
Czasem przetestowanie prywatnych metod wywoływanych przez publiczne powoduje, że pisanie testów jest dużo szybsze, że trzeba ich mniej i trudniej jest coś przeoczyć.


Nie moge się z tym zgodzić. Bo sugerujesz, że wystarczy sprawidzić dowolne wejście publicznego API i potem sprawdzić prywatne metody i problem z głowy. W ten sposób przetestowałeś TYLKO wewnetrzne metody a nie API.
Testy API i tak musisz napisać i tak. Więc pisanie testów dla metod prywatnych zawsze będzie trwało dłużej, a nie przyniesie moim zdaniem żadnych benefitów. Każda, najwet najmniejsza zmiana w implementacji pociągnie za sobą refactoring testów. Bez względu na to czy publiczne API po testach dalej działa czy nie.

Cytat(vokiel @ 26.06.2015, 11:13:00 ) *
Testowanie prywatnych powoduje, że testy będą czytelniejsze i lepiej zorganizowane, łatwiej jest też obsłużyć wszystkie scenariusze.


Nieprawda. Testując metody prywatne nie jesteś w stanie przetestować ani jednego scenariusz, bo scenariusz opiera się w 100% na tym co weszło do metody publicznej i jaką ścieżkę wybrała metoda publiczna. Absolutnie nie przetestujesz w ten sposób różnych scenariuszy a wyłącznie jedną metodę prywatną w oderwaniu od rzeczywistości. Chyba, że odwzorujesz w testach to, co robi metoda publiczna, ale wtedy wracamy do punktu wyjścia. Po co to robić, skoro już przetestowałeś metodę publiczną i ona odpaliła każdy scenariusz?

Cytat(vokiel @ 26.06.2015, 11:13:00 ) *
Ostania rzecz odnośnie samego code coverage, na które też trzeba uważać żeby nie pisać testów pod nie. Jest pokusa aby testy przeszły przez wszystkie gałęzie kodu i było 100% green, ale nie w tym rzecz. Bo same puste przejścia wcale nie gwarantują przetestowania ich efektów. To, że kod się wywołał nie oznacza, że wynik tego wywołania został sprawdzony. Już widziałem testy, gdzie Config->get() było wywoływane dziesiątki/setki razy, dobry CRAP ale co z tego, jak sama klasa Config nie była przetestowana jednostkowo, indywidualnie i nie zostały sprawdzone żadne przypadki brzegowe. Wystarczyło w configu w jednym polu wpisać -1 i fail. Setki wywołań pośrednich na niewiele się zdały.


Ale to nie problem code-coverage. Testy jednowstkowe powinny testować jednostę. I coverage powinien być tylko na testowanej klasie a nie na jej zależnościach. Twojej klasy nie interesuje, czy metoda get na obiekcie config działa a tym bardziej czy pokrywa jakiś kod. To się da zamockować a sam config sprawdzić własnymi testami. Wtedy nie masz takich problemów.
I wtedy code-coverage pozwala Ci szybko i łatwo wykryć martwy kod. Jeśli coś się nie wykonje to można to usunąć. Proste.

W PHPUnit jest od tego annotation @covers. Wtedy ustalasz co ma być pokrywane przez testy. Wtedy code-coverage ma rece i nogi. Stwierdzenie, że każdy test pokrywa wszystko to IMO głupota, bo niczego Ci nie daje. Dosłownie niczego. A jesli masz testy funkcjonalne to tym bardziej nie interesuje ich code-coverage. Je interesuje czy całość działa a nie czy jednostka działa.
vokiel
Odpowiem po kolei, potem tl;dr ;-)

Cytat(Tuminure @ 26.06.2015, 11:38:33 ) *
Nawet po przetestowaniu każdej metody prywatnej oddzielnie, powinieneś przetestować wspomniane wszystkie kombinacje dla metody publicznej.

Chyba nie do końca się zrozumieliśmy. Nie postuluję za tym, żeby testować wszystkie prywatne jak leci i zrezygnować z testów publicznych. Twierdzę tylko, że czasem testowanie metod prywatnych jest wygodniejsze i bardziej czytelne oraz, że powoduje, że aby wszystko dobrze przetestować należy napisać mniej testów.

Cytat(Tuminure @ 26.06.2015, 11:38:33 ) *
Błędów, które są nieistotne dla działania klasy.

Zależy co robią te metody prywatne. Poza tym żadne błędy nie są mile widziane.

Cytat(Tuminure @ 26.06.2015, 11:38:33 ) *
Oznacza to jedynie tyle, że test jest źle napisany (lub jeżeli kolizje błędów zwracają ZAWSZE poprawne wyniki, to test jest napisany dobrze, a wspomniane błędy są nieistotne dla działania).

Jeśli mamy metodę publiczną pubA(), która wywołuje wewnątrz kilka metod prywatnych przekazując wyniki działania jednej do drugiej. Może się zdarzyć tak, że metoda prywatna privA() da niewłaściwy wynik, który przekazany do privB(), a jej wynik przekazany do privC() finalnie zwróci oczekiwany wynik (np false). Do czasu, gdy te metody tylko operują na danych wejściowych jest ok - tak jak piszesz, nie ma to znaczenia, bo finalnie jest tak jak powinno być (o ile jest to powtarzalne, a nie dzieje się tak w jednym przypadku na 10). Natomiast jeśli któraś z tych metod przekazuje te dane dalej to może pojawić się problem.

Poza tym odnalezienie błędu jest bardziej problematyczne, bo gdy nie przejdą testy to do sprawdzenia są wszystkie wykorzystane przez publiczną metody prywatne. Nie jest to oczywiście jakiś wielki problem, ale zdecydowanie łatwiej i czytelniej to widać jeśli phpunit pokaże to w samych testach (nie przechodzą testy privC()). Niż, gdy pokaże, że nie przechodzą testy pubA() - i błąd może być w każdej z prywatnych metod - sprawdź ręcznie sam.

I jeszcze jedno odnośnie ilości testów.
W przykładzie powyżej użyłem sprawdzenia czy zmienna jest większa, mniejsza czy równa zero. Jeśli takie warunki będziemy mieli w trzech prywatnych metodach to aby przetestować wszystkie przypadki za pomocą testów tylko publicznej metody musimy wygenerować 3x3x3 = 27 przypadków. Testując prywatne metody oddzielnie mamy tylko 3x3 = 9. Oczywiście nie zawsze tak będzie, niemniej są przypadki gdy oddzielne testowanie metod prywatnych zmniejsza ilość testów.


Cytat(Xelah @ 26.06.2015, 12:23:23 ) *
I to dalej jest nieistotne z punktu widzenia API. Jeśli masz błąd w prywatnej metodzie a API dla dowolnego wejścia produkuje poprawne wyjście, to znaczo to, tyle, że masz błąd w teście. A nie w kodzie.

Zgadzam się, z punktu widzenia API jest nieistotne, ale o ile jest tak jak pisałem wyżej, że prywatne tylko obrabiają dane. Oraz, gdy tak jest dla dowolnego wejścia - tzn błąd jest na swój sposób niezawodny.

Cytat(Xelah @ 26.06.2015, 12:23:23 ) *
A co Ci mówi fakt, że metoda prywatna zwróciła to, co trzeba? Z punktu widzenia API? Nic. Dosłanie nic. Na pewno nie mówi Ci, że API działa. API i tak będzie przechodziło przez całą ścieżkę i ważne jest, żeby to działało.
API testujesz tak, żeby osiągnąć 100% code-coverage. Wtedy wiesz, że sprawdziłeś każdą ścieżkę (oczywiście nie gwarantuje Ci to braku błędów). Powinieneś też sprawdzić warunki brzegowe, ale to dalej nie ma związku z metodami prywatnymi.

Daje to, że wiemy że ta metoda działa poprawnie. Gdy testy metody publicznej się wysypią to wiemy, że tutaj jest wszystko ok i szukamy błędu gdzie indziej. Nie twierdzę, że testy metod publicznych mają zastąpić testy API ale, że są pomocne.

Testowanie metod prywatnych zwiększa szansę na brak błędów, zmniejsza współczynnik CRAP.

Cytat(Xelah @ 26.06.2015, 12:23:23 ) *
Nie moge się z tym zgodzić. Bo sugerujesz, że wystarczy sprawidzić dowolne wejście publicznego API i potem sprawdzić prywatne metody i problem z głowy.
W ten sposób przetestowałeś TYLKO wewnetrzne metody a nie API.

Takie pytanie, czy traktujesz tutaj API na równi z metodami publicznymi, jako zamiennik słowa?
IMHO od tego są właśnie testy jednostkowe - żeby sprawdzić wejścia publicznych i prywatnych metod. Potem są testy integracyjne do sprawdzania pozostałych zależności i przypadków.

Cytat(Xelah @ 26.06.2015, 12:23:23 ) *
Testy API i tak musisz napisać i tak. Więc pisanie testów dla metod prywatnych zawsze będzie trwało dłużej, a nie przyniesie moim zdaniem żadnych benefitów. Każda, najwet najmniejsza zmiana w implementacji pociągnie za sobą refactoring testów. Bez względu na to czy publiczne API po testach dalej działa czy nie.

Jednym słowem TDD.

Cytat(Xelah @ 26.06.2015, 12:23:23 ) *
Nieprawda. Testując metody prywatne nie jesteś w stanie przetestować ani jednego scenariusz, bo scenariusz opiera się w 100% na tym co weszło do metody publicznej i jaką ścieżkę wybrała metoda publiczna.
Absolutnie nie przetestujesz w ten sposób różnych scenariuszy a wyłącznie jedną metodę prywatną w oderwaniu od rzeczywistości.

Racja, użyłem złego zwrotu. Nie miałem na myśli scenariuszy jako ścieżek przejścia tylko jako możliwych przypadków/kombinacji.

Chyba właśnie o to chodzi w testach jednostkowych, nie?
Kiedyś widziałem w czyjejś prezentacji, że unit należy traktować jako najmniejszy możliwy black box - czarną skrzynkę, która dostaje coś na wejście i wypluwa jakiś wynik. I że właśnie unit testy służą testowaniu takich black box'ów w oderwaniu od otoczenia. Nie ważne jakie inne metody ją wywołują, czy jest publiczna, prywatna czy chroniona. Ważne jakie daje wyniki na podane dane wejściowe.

Cytat(Xelah @ 26.06.2015, 12:23:23 ) *
Ale to nie problem code-coverage. Testy jednowstkowe powinny testować jednostę. I coverage powinien być tylko na testowanej klasie a nie na jej zależnościach. Twojej klasy nie interesuje, czy metoda get na obiekcie config działa a tym bardziej czy pokrywa jakiś kod. To się da zamockować a sam config sprawdzić własnymi testami. Wtedy nie masz takich problemów.
I wtedy code-coverage pozwala Ci szybko i łatwo wykryć martwy kod. Jeśli coś się nie wykonje to można to usunąć. Proste.


Właśnie o to mi chodziło, że czasem ktoś testuje tylko publiczne API pod kątem coverage, które we wspomnianym przeze mnie przypadku pokaże 100% pokrycia i dziesiątki przejść testów. Pisze testy dla innych klas, zagląda do coverage i widzi 100% dla Config - no to ok, jazda z pozostałymi testami, a takie coverage nic nie oznacza, bo de facto sprawdza tylko wycinek rzeczywistości. To tylko takie moje uczulenie dla tych, którzy rzucili się na coverage jakby to był wyznacznik jakości testów.


tl;dr
Uważam, że można testować metody publiczne. Nie należy jednoznacznie określać, że wystarczy przetestować publiczne wraz ze wszystkimi ścieżkami przejścia (100%) coverage i sprawa załatwiona. Owszem czasem napisanie testów do wielu metod prywatnych trwa dłużej, wymaga więcej refaktoringu przy zmianach w kodzie, ale daje większą stabilność i pozwala szybciej odszukać błąd na podstawie samego wyniku phpunit. Bywa, że napisanie testów do metod prywatnych powoduje, że testów jest mniej, niż w przypadku testowania ich za pomocą publicznych.

Spotkałem się nawet z podejściem testowania metod w jak największym oderwaniu od otoczenia - mockować jak najwięcej, żeby wyniki jednych nie wpływały na wynik testu jednostkowego danej metody. Oczywiście nie ma co popadać w skrajności i testować wszystkie metody prywatne, a potem je mockować przy testach metod publicznych. Niemniej każda funkcja/metoda to ten unit, który jest tym najmniejszym black box'em, który można przetestować. Ważne, żeby zachować umiar i nie popadać ze skrajności w skrajność. Dodatkowe testy metod prywatnych nie zaszkodzą, mogą co najwyżej zabrać trochę czasu na ich pisanie i modyfikację.
Dejmien_85
Cytat(darek334 @ 25.06.2015, 20:40:47 ) *
ogólnie zajmuję się informatyką od 20 lat


Pochwal się od jak dawna jesteś zawodowym programistą (tworzącym oprogramowanie - tylko proszę o staż z programowania serwisów/platform/systemów, tworzenie stron www na CMS-ach w to nie wliczamy) - informatycy mi się bardzo źle kojarzą, to takie ogólne pojęcie "od wszystkiego i do niczego". Oczywiście nie mówię, że jesteś taką osobą, słowo "informatyk" jest po prostu bardzo podejrzane. ; )

A co do testowania oprogramowania - ten temat już wielu przerabiało i ogólnie dobrym nawykiem jest testowanie interfejsów (metod publicznych), tj. wyjścia i wejścia. Dla metody prywatnych nie trzeba pisać testów, ponieważ utrudniają późniejsze zmiany i modyfikacje kodu - w przypadku zmiany implementacji danej funkcjonalności (metoda publiczna) wysypują się testy metod prywatnych mimo tego, ze wejście/wyjście zwraca poprawne wyniki.
Xelah
Odnoszę wrażenie, że zupełnie inaczej widzimy temat unit i ogólnie testów.

Dla mnie coverage to bardzo istotna sprawa. Właśnie po to, żeby sprawdzić, czy kod który mam nie jest martwy i czy aby nie ma tam jakiegoś poważnego kuku. I tylko tyle. Oczywiście, że to nie ma znaczenia "sensu stricto", ale daje jeśli trzymasz się zasady, że cocerage jest tylko klasy ktrórą testujesz (a testuje zawsze wyłącznie jedną klasę), to łatwo sprawdzić, czy każda linijka kodu ma żyje i ma się dobrze. To, czy produkuje prawidłowy output to już problem testów i nie ma nic wspólnego z coverage.
I w pełni się zgodzę, że code-coverage sam w sobie nic nie znaczy. Fakt, że każda linijka kodu jest wykonana nie znaczy że nie ma problemu, albo, że kod działa prawidłowo. Bo to nie jest cel code-coverage.

Jeśli chodzi o API, to tak, mówię tutaj o metodach publicznych. Bez znaczenia jest czy metoda publiczna jest definiowana przez interfejs czy nie. Jeśli jest publiczna to staje się automatycznie częścią API (Bardzo dobrze widać to we frameworkach, które ze względu na BC utrzymują API, które czasami powoduje palpitacje od samego patrzenia).
I uważam, że najważniejsze są tylko metody publiczne, bo to je możemy dowolnie wywoływać i to one muszą dla danego wejścia produkować spodziewany wyjście. Metody prywatne takiej miżliwości nie mają. Nie mogę ich dowolnie wywoływać z dowonlymi parametrami i w dowolny sposób. Bo o tym czy i jak będą wykonane decuduje publiczne API.
Jedyne, co akceptuję w przypadku metod prywatnych to sprawdzanie, co weszło do mock-a (na przykład expects()->method()->with() w PHPUnit). Bo to rzeczywiście może być ważne, biorąc pod uwagę, że obiekt jest mockowany i przyjmuję, że zadziała, o ile na wejściu dostanie to co trzeba. Jak nie, to problem wykryję najwcześniej na poziomie testów funkcjonalnych.

Co do ścieżek wykonania to chyba ja tym razem byłem nie wyraziłem się jasno. Moim zdaniem nie chodzi o to, żeby testować każdy możliwy input, po to, żeby sprawdzić każdy możliwy output. Nawet każdy poradnik odnośnie testów na wstępie mówi jedno: Zawsze możesz napisać więcej testów. Testy mają za zadanie sprawdzić, czy jeśli podamy prawidłowy input to dostaniemy spodziewany output.
I co najważniejsze - testy nie sprawdzają, czy kod nie ma błędów. Od tego są testy funkcjonalne. Testy jednostkowe, IMO, pozwalają na szybszy i prostszy design danego komponentu (TDD). Fakt, że jeśli mamy błąd to powinniśmy napisać test, który nie przejdzie, a potem naprawić kod - to zupełnie inna bajka.
Podają taki bardziej hardcorowy przykład. Wyobraź sobie, że dostajesz na wejściu string. W środku następuje rozbicie go na pojedyńcze wyświetlalne znaki. Czyli co, mam napisać testy dla każdej możliwej kombinacji znaków, żeby sprawdzić czy działa? Oczywiście, że nie. Mogę sprawdzić wartości brzegowe, ewidentnie zły input (np. int zamiast string) i jeden zwykły input. Inaczej szybko przejdziemy od pisania szybkich testów do pisania tysięcy testów pod jedną klasę, bo przecież można na wejściu podać cokolwiek i trzeba sprawdzić czy zawsze zadziała. IMO to nie o to chodzi. Sprawdzić trzeba jedną wartość spodziewaną i warunki brzegowe. A odatkowe testy zawsze można dopisać później, jeśli zajdzie taka potrzeba.

Apropos sensu testów metod prywatnych. Kilka miesięcy temu dyskutowaliśmy to podczas code-review i zadaliśmy to pytanie Sebastianowi Bergmann-owi. Pokierował nas prosto do swojego posta z przed 5 lat:
https://sebastian-bergmann.de/archives/881-...r-Privates.html

Dwa ostatnie akapity podsumowują temat dosyć dobrze.

I ja też jestem zdania, że metody prywatne można testować wyłącznie w bardzo specyficznych przypadkach. Zazwyczaj jednak nie ma takiej potrzeby.
szubi95
Chwilkę nie odwiedzałem, a tu kurcze dyskusja na całego ^^

Mam jeszcze pytanko i chciałbym to pokazać na kodzie:

  1.  
  2. public function init(Array $route, Array $request=null)
  3. {
  4. $final = '';
  5. $temporary = [];
  6. if (!empty($route)) {
  7. foreach ($route as $field => $value) {
  8.  
  9. if (is_string($value)) {
  10. $this->$field = filter_var($value, FILTER_SANITIZE_STRING);
  11. } elseif (is_array($value)) {
  12. foreach ($value as $index => $array_value) {
  13. if (is_int($index)) {
  14. $type = gettype($array_value);
  15.  
  16. switch ($type) {
  17. case 'string':
  18. $value = filter_var($array_value, FILTER_SANITIZE_STRING);
  19. break;
  20. case 'integer':
  21. //echo 3;
  22. break;
  23. }
  24. }
  25.  
  26. $temporary[$index] = $value;
  27. }
  28. $this->$field = $temporary;
  29. }
  30.  
  31. }
  32.  
  33. $class = $this->checkControllerClass($this->module, $this->controller);
  34.  
  35. if (true == $class) {
  36. $this->checkMethod($this->action);
  37. $this->checkParameters($this->params);
  38.  
  39. echo $this->module;
  40. echo $this->controller.'<br>';
  41. echo $this->action.'<br>';
  42.  
  43. if (!empty($this->module) && !empty($this->controller && !empty($this->action))) {
  44. $final = $this->module.'\\'.$this->controller;
  45. }
  46. }
  47. }
  48.  
  49. return $final;
  50. }


Mam załóżmy taką metodę i taki test:

  1.  
  2. public function testInit()
  3. {
  4. $route = [
  5. 'module' => 'Lib\\Controllers',
  6. 'controller' => 'User',
  7. 'action' => 'show',
  8. 'params' => [
  9. 1 => 5,
  10. 2 => 'param'
  11. ]
  12. ];
  13.  
  14. $request = [
  15. 'cos' => 'cos',
  16. 'cos1' => 'cos1'
  17. ];
  18.  
  19. $string = $this->page_controller->init($route, $request);
  20.  
  21. $this->assertCount(4, $route);
  22. $this->assertArrayHasKey('module', $route);
  23. $this->assertArrayHasKey('controller', $route);
  24. $this->assertArrayHasKey('action', $route);
  25. $this->assertArrayHasKey('params', $route);
  26.  
  27. $this->assertInternalType('string', $route['module']);
  28. $this->assertInternalType('string', $route['controller']);
  29. $this->assertInternalType('string', $route['action']);
  30. $this->assertInternalType('array', $route['params']);
  31. $this->assertInternalType('string', $string);
  32.  
  33. $this->assertNotEmpty($string);
  34. $this->assertRegExp('#(^[\a-zA-Z]*)$#', $string);
  35. }
  36.  


Czy dobrze rozumiem testowanie argumentów, czy to w ogóle ma jakiś sens? Kolejnymi przypadkami są teraz np. puste tablice na wejście i też sprawdzenie.

I druga sprawa, co zrobić jeśli funkcja nic nie zwraca, a np tworzy obiekt i wywołuje jego metodę z argumentami jak to ma miejsce właśnie w przypadku PageController'a? Po prostu testować same jej argumenty?


Pozdrawiam,
szubi
Xelah
IMO to co zrobiłeś w ogóle nie ma sensu. Testujesz tablicę, którą sam utworzyłeś na potrzeby testu. Co dokładnie Ci to daje? Nic. Masz przetestować kod, który napisałeś a nie sam test.

Abstrachując od tego, czy sam kod ma ręce i nogi (a niestety nie ma), to sprawdzasz wyjście funkcji na podstawie wejścia.

  1. public function testReturnsCorrectOutput()
  2. {
  3. $request = [...]
  4. $route = [...]
  5. $controller = new PageController();
  6.  
  7. $expectedOutput = '....';
  8. $actualOutput = $controller->init($route, $request);
  9.  
  10. $this->assertSame($expectedOutput, $actualOutput);
  11.  
  12. // Jeśli na wyjściu spodziewasz się istniejącej klasy
  13. $this->assertTrue(class_exists($actualOutput));
  14. }


Jeśli chcesz przetestować czy zachowanie jest prawidłowe jeśli na wejściu jest dostaniesz nieprawidłowe dane to robisz to dokładnie w ten sam sposób. Generalnie Twoje testy idą w złym kierunku.

Nie wspominając już o fakcie, że sam kod pozostawia wiele do życzenia. Dla przykładu przekazujesz do funkcji dwie tablice a potem się martwisz, czy mają w środku to, co trzeba. Nie prościej przekazać dwa obiekty? Route i Request? Wtedy masz pewność, że na pewno jest tam to, co chcesz. Typy prymitywne można czasami przekazywać jako parametry do konstruktora, ale w tym przypadku nie ma najmniejszego uzasadnienia, żeby przekazywać tablicę.
Wtedy Twój kod będzie o wiele prostszy, czytelniejszy i łatwiejszy do przetestowania.

Generalnie jedna ważna uwaga. Jeśli nie jesteś w stanie czegoś przetestować, albo nie masz pojęcia jak to zrobić, to zazwyczaj wyraźny znak, że z Twoim kodem jest coś nie tak i należy przemyśleć czy nie należałoby go zmienić.
szubi95
I właśnie to mi się nie zgadzało, teraz już chyba rozumiem smile.gif

no tak, o tym nie pomyślałem, imo chciałem zrobić dwa niezależne od siebie komponenty(łatwe niezależne wykorzystanie, a tutaj trochę ograniczam. Z drugiej strony jednak to cała "biblioteczka" powinna być niezależna - to by bardziej miało sens.


  1. public function init(Router $router, Request $request=null)
  2. {
  3. $route_array = $router->resolve();
  4.  
  5. $bool = $this->checkControllerClass($route_array['module'], $route_array['controller']);
  6.  
  7. if ($bool) {
  8. //...
  9. }
  10. }


Dziękuję i pozdrawiam,
szubi
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.