Testy jednostkowe w PHPUnit mojego PHP Framework'a

Witam! Zacznę od początku. Napisałem niedawno aplikację Opiner.me i potem na jej podstawie własny PHP Framework. Generalnie nie wierzę, że ktoś będzie go używać poza mną, ale zostały mi do dokończenia testy jednostkowe w PHPUnit. Chyba wypadałoby dopisać testy dla klas rdzenia (Core) framework'a... Zmierzając do sedna. Czy napisane już testy wymagają poprawy? Wydaje mi się, że są sensownie napisane, ale nie mam pewności. Czy zawierają rażące błędy, które wymagają poprawy?

EEQ napisał(a):

Witam! Zacznę od początku. Napisałem niedawno aplikację Opiner.me i potem na jej podstawie własny PHP Framework. Generalnie nie wierzę, że ktoś będzie go używać poza mną, ale zostały mi do dokończenia testy jednostkowe w PHPUnit. Chyba wypadałoby dopisać testy dla klas rdzenia (Core) framework'a... Zmierzając do sedna. Czy napisane już testy wymagają poprawy? Wydaje mi się, że są sensownie napisane, ale nie mam pewności. Czy zawierają rażące błędy, które wymagają poprawy?

Ogólnie testy napisane po fakcie (i sam kod) nie będą tak dobre, jak testy napisane wcześniej (chyba że piszesz dokładnie to samo któryś raz).

Widzę trzy foldery w katalogu tests/. Weźmy pierwszy MainPageValidatorTest. Pierwsze co mi się wrzuca w oczy to to, że to chyba powinny być kilka testów zamiast jednego dużego. Np. masz:

$validator->validate('Name', $token);
$valid = $validator->isValid();

$this->assertTrue($valid);

$validator->validate('', $token);
$valid = $validator->isValid();

$this->assertFalse($valid);

$validator->validate('123456789012345678901', $token);
$valid = $validator->isValid();

To się aż prosi, żeby rozdzielić to na osobne testy, np tak:

#[Test]
public function regularNameIsValid(): void {
  $validator->validate('Name', $token);
  $this->assertTrue($validator->isValid());
}

#[Test]
public function emptyNameIsNotValid(): void {
  $validator->validate('', $token);
  $this->assertFalse($validator->isValid());
}

#[Test]
public function tooLongNameIsNotValid(): void {
  $validator->validate('123456789012345678901', $token);
  $this->assertFalse($validator->isValid());
}

Jednocześnie trochę rozumiem czemu zachowałeś to w jednym teście - setup validatora jest skomplikowany, przez to że korzysta z Csrf tokenu, i dużo kodu zajmuje jego setup - trzeba by go wydzielić albo mieć funkcję pomocniczą. To jest jeden z powodów czemu pisanie testów najpierw jest lepsze niż potem. Gdybyś napisał testy najpierw - najpewniej napisałbyś dużo prostsze testy. Ale ponieważ stworzyłeś walidator i csrf token najpierw, to powstał Ci design który jest trudny do przetestowania. Gdybyś najpierw napisał testy, one wywarłyby na Ciebie pewnego rodzaju wpływ, żebyś stworzył kod który jest bardziej testowalny.

Kolejny test UserRepositoryTest - wstyd się przyznać - ja autentycznie nie wiem co ten test robi Nie potrafię się domyślić o co w nim chodzi.

Kolejny test MainPageBrowserTest - wygląda jak połączenie testu integracyjnego i akceptacyjnego, taki pół na pół. testMainPageTitle() ma tą samą wadę co wcześniej, czyli testowanie dwóch rzeczy w jednym teście - spokojnie można by to rozbić na dwie. Test akceptacyjny który sprawdza czy title HTML jest ustawiony, moim zdaniem okej. Test akceptacyjny który sprawdza status code... hmm... no można polemizować. Jedyna rada jaką mam to to, że testy które stawiają przeglądarkę z reguły działają wolniej niż jednostkowe. Jednostkowych możesz uruchomić tysiące w kilka sekund, takie które podnoszą przeglądarke uruchomisz kilka/kilkanaście w minute - takie testy dobrze jest zrównoleglić. Niestety phpunit nie działa dobrze w równoległe testy, trzeba kombinować z paratest (osobna biblioteka) ale ona też ma swoje wady i nie działa dobrze. Można się zastanowić albo nad sposobem żeby zrównoleglić testy przeglądarkowe w PHP, albo możesz odpalić testy akceptacyjne w innej technologii, takiej w której paralelizacja jest prosta (np. Python, JS, etc.). Takie testy nie muszą znać interfejsu programistycznego frameworka, więc nie ma problemu żeby były w innym języku.

PS: Po co Ci customowy autoload.php, jeśli korzystasz z Composera?

hyper-stack napisał(a):

@Riddle na poczatku, na to powinienes przede wszystkim zwrocic uwage:

Kopiuj

public function emptyNameIsNotValid(): void {
  $validator->validate('', $token);
  $this->assertFalse($validator->isValid());
}

ktory to psr?

A jakie to ma znaczenie? Ustawi sobie autoformat w IDE i nie musi o tym myśleć.

hyper-stack napisał(a):

Jesli gosc nie nabierze wlasciwych nawykow na poczatku, to cos takiego sprawi, ze jego cv natychmiast wyladuje w koszu.

Jeśli ktokolwiek odrzuci kandydata przez taką błachostkę, to szczerze mówiąc wyświadczył mu przysługę, bo pracować z takim zespołem to musi być katorga.

Riddle napisał(a):

Jeśli ktokolwiek odrzuci kandydata przez taką błachostkę, to szczerze mówiąc wyświadczył mu przysługę, bo pracować z takim zespołem to musi być katorga.

Powiedz to tym, ktorzy n-ty miesiac z rzedu zalegaja z rata kredytu, a zonki z papierami rozwodowymi w reku "szykuja sie do sprintu w blokach startowych".

Zejdz na Ziemie czlowieku.

@hyper-stack Ale Ty odrzucasz kogoś na rekrutacjach za nawiasy w złych liniach, czy sam byłeś odrzucany i przestrzegasz innych?
Jeśli nawet piszesz kod jako zadanie na rekrutację w domu to masz IDE, masz CodeSniffer.
Jak piszesz live codding to już trzeba być wyjątkowym zwyrolem, żeby zwrócić na to uwagę.
Ja jestem tu po części bezstronny, ale nie karałbym nikogo na forum, za próbę pomocy i napisanie nawiasu niezgodnie z PSR. Jak piszę nieraz z telefonu 2 linijki czy 3 to też nie zwracam uwagi na spacje czy nawiasy.

jurek1980 napisał(a):

@hyper-stack Ale Ty odrzucasz kogoś na rekrutacjach za nawiasy w złych liniach, czy sam byłeś odrzucany i przestrzegasz innych?

Oczywiscie, ze nie odrzucam kogos za nawiasy zlych liniach, ale ...
Jak mam kandydata Ukrainca i Polaka o porownywalnych skilach, wymaganiach i umiejetnosciach wybiore zawsze Polaka.
Podobnie z "nawiasami".
Nie mam dylematu kogo wybrac sposrod dwoch podobnych kandydatow spelniajacych moje kryteria w oczekiwanych scope'ach.
Jesli jeden "stawia" a drugi nie, wybor jest dla mnie oczywisty.

Programisci, bezwzglednie wykorzytywali "swoj" rynek w przeszlosci.
W obecnej sytuacji teraz oni sa bezwglednie wykorzystywani, a jak sam wiesz jest z czego wybierac.
Ludzie siedza bez roboty mimo imponujacych skili, wiec ostatecznym kryterium odsiewu sa wlasnie "pi.e..r...d....oly".
Nie licz na empatie, urode, auto sprzedaz lub marke osobista.

Po tym jak bez skrupolow, zostawiales (oczywiscie nie Ty personalnie bo tego przeciez nie wiem) firme/kontrahenta 3 - 4 razy w roku bo Twoj rozwoj osobisty/zawodowy nie byl dla Ciebie satysfakcjonujacy, bo nie podawali truskawek w owocowe czwartki, bo inna firma placila "2 PLN" wiecej (mozna by tak wymieniac i wymiena) na inne niz opisalem wyzej traktowanie raczej nie licz.

Tak jak bylo, "to se ne wrati", teraz nie dostaje sie roboty wlasnie przez "nawiasy".

jurek1980 napisał(a):

@hyper-stack Jeśli nawet piszesz kod jako zadanie na rekrutację w domu to masz IDE, masz CodeSniffer.

Natomiast w mojej ocenie wiedzac ze stawiasz sie w roli mentora (oczywiscie nie dotyczy to Ciebie), majac swiadomosc ze wieloletnie korzystanie na codzien z CS'a lub stan'a w momencie pozbawienia Cie tych narzedzi (telefon) sprawia ze pomagajac w jednym, rownoczesnie, bardzo nie pomagasz w drugim ... reszte sobie dopowiedz.

jurek1980 napisał(a):

Jak piszesz live codding to już trzeba być wyjątkowym zwyrolem, żeby zwrócić na to uwagę.

To prawda, moim zdanie w ogole wykorzytywanie live codingu jako miarodajna wartosc do oceny to zwyrolstwo. Zawsze chce wyciagnac z kandydata najlepsze co ma do zaoferowania, a wiekszosci przypadkow ludzie sa najbardziej efektywni po prostu jak sie ich zostawi w spokoju.

jurek1980 napisał(a):

Ja jestem tu po części bezstronny, ale nie karałbym nikogo na forum, za próbę pomocy i napisanie nawiasu niezgodnie z PSR. Jak piszę nieraz z telefonu 2 linijki czy 3 to też nie zwracam uwagi na spacje czy nawiasy.

Wybacz, ale Ty to nie @Riddle (chociaz kto wie, ale nie znam Cie osobiscie).
W przypadku @Riddle aura "akademika" ktora sobie tworzy na tym forum od wielu lat (tak na marginesie, niezaprzeczalna, nawet wg mnie) zobowiazuje go jednak do pewnych standardow, a wstawki w stylu:

A jakie to ma znaczenie? Ustawi sobie autoformat w IDE i nie musi o tym myśleć.

lub:

Jeśli ktokolwiek odrzuci kandydata przez taką błachostkę, to szczerze mówiąc wyświadczył mu przysługę, bo pracować z takim zespołem to musi być katorga.

swiadcza o coraz wiekszym i wlasciwym dla "akademikow" oderwaniu od rzeczywistosci. Nie jest to przytyk, ale zal bo @Riddle to jedyne co zostalo dobrego na tym forum po czasach swojej swietnosci.