skokoban / LibrinostriCzDownloader

Application for batch downloading PDF files from website librinostri.catholica.cz.
0 stars 0 forks source link

Mockito nepošle predom pripravený objekt #1

Open skokoban opened 1 year ago

skokoban commented 1 year ago

Čau, prosím ťa o radu ako si poradiť s týmto testom. Cez mockito poviem aby pri zavolaní metódy saveToHDD dodal moj pripravený objekt, aby sa nespúšťala tá originál metóda.Ale ono to ide stále do tej normál metódy. tento riadok to je: https://github.com/skokoban/LibrinostriCzDownloader/blob/b5cb5c3f875c04c4f1acbb57f5eb758a6899d846/src/test/java/tools/DownloaderTest.java#L60 Podobnú vec som písal v tomto istom projekte akurát som testoval inú triedu a tam to funguje podla mojho ocakavania: https://github.com/skokoban/LibrinostriCzDownloader/blob/b5cb5c3f875c04c4f1acbb57f5eb758a6899d846/src/test/java/main/LibrinostriTest.java#L50

petoju commented 1 year ago

@skokoban nemyslim si, ze to funguje.

Mockito ti vyrobi object (napr. ten librinostriMock). Na nem si potom mozes zavolat parseDownloadLinks a vyrobi ti to, co chces. Iba ty si robis uplne inaci objekt tu: https://github.com/skokoban/LibrinostriCzDownloader/blob/b5cb5c3f875c04c4f1acbb57f5eb758a6899d846/src/test/java/main/LibrinostriTest.java#L52 keby sa pouzil librinostriMock, tak by to fungovalo s mockem.

Rovnako pri bookMocku - ten sa nikde nepredava, tak sa ten mock ani nepouzije a je vlastne nanic.

Na test, ci ti to funguje je aspon pri pisani testu vhodne preklopit tam assertion - ci to spadne jak chces a ma naozaj to, co chces.

Ty si inac v LibrinostriTest v mocku pouzivas vrateni prazdneho rssDocument - ono je lepsi tam vratit neco, co sa podoba produkcnemu. A idealne neco, kde bude jasne, ze to neni z produkcie - aby ti bylo jasne, ze netestujes co chces testovat.

Inac s tym mockovanim SAXBuilderu - toto ta dovede k temu, ze to treba napisat trochu inac, aby sa to dalo testovat. Napr. ze sa bude SAXBuilder davat jak volitelny parameter v konstruktori. Alebo bude tato trida iba spracovavat RSS data, ale dostane ich od nekoho druheho.

Dalsi poznamky (iba take postrehy):

  1. Volba jak "chceme elementy od 4. dalej" typicky nema velky vyznam. Jakoze nechceme prve 4, lebo maju neco specificke. Vime co? Proc to nepreskocit tak? Totiz do XML lahko nekdo prida element na zacatek a potom sa s tym takto nevysporadas.
  2. Mozno by to cele islo zjednodusit pouzitim XPath namisto pruchodu cez SAX. V SAX lude skoro nepisu, chce to spravit si jednoduchy stavovy automat. V XPath vhodne specifikujes, co chces vybrat a on to najde - ma to samozrejme nevyhody jak ze musi byt cely dokument v pamati.
skokoban commented 1 year ago

Dakujem za rady.

Inac s tym mockovanim SAXBuilderu - toto ta dovede k temu, ze to treba napisat trochu inac, aby sa to dalo testovat. Napr. ze sa bude SAXBuilder davat jak volitelny parameter v konstruktori. Alebo bude tato trida iba spracovavat RSS data, ale dostane ich od nekoho druheho. Na tento problem, ze sa mi tazko testuje narazam uz nejaku dobu. Testy mam zlozite lebo musim vytvorit plno objektu, ktore ta testovana metoda potrebuje. Stale ale nechapem kde je jadro problemu. Myslel sem, ze si pomozem tym mockovanim. Neni mi jasne proc mi pomoze ked objekt predam testovanej tride cez konstruktor oproti temu ked si zavolam testovanu metodu z testovanej tridy s tym, ze objekty tam dodam nejako inako. V kazdem pripade mi pride, ze tie objekty musim v testovacej triede vytvorit nech uz ich predavam ako parameter do konstruktora alebo inak - ako som to robil doteraz. Myslim aj, ze to automaticke spustanie metod, ktore mam napr. v konstruktore Librinostri() tiez nieje dobre pre testovanie- ked sa mi tam zbytocne spustaju vselijake metody. - Tuto vec som zas pochytil na videu z prednasky programovania na STU. Ten ucitel daval ziakom zadanie, ze nechce vidiet ziadne volanie metod na objekte ale ma sa to same spravit len zavolanim konstruktora. Ked tak nad tym rozmyslam tak ma napadlo, ze im to povedal len z edukativnych ucelov a ked sa pohnu dalej v ucive tak im povie, ze spravne sa to ma rovit inak. Ked si k tomu budem chciet precitat nejaky tutorial, je to ten termin dependency injection?

S tym, ze to nefunguje mas samozrejme pravdu- netestovalo mi to co som chcel. Musim to mockito este poskusat na jednoduchsich prikladoch lebo nerozumiem zatial ako to ma fungovat.

petoju commented 1 year ago

Testy mam zlozite lebo musim vytvorit plno objektu, ktore ta testovana metoda potrebuje.

No, to je prave cele o tem, jak to navrhnut, aby islo o minimalny interface. Ty to mas dost prepletene.

Priklad jak to "rozmotat":

Teda chcem stahovat to, co bude v RSS. Tu to opisem mozno trochu detailnejsi:

  1. Nekdo prvy dostane URL RSS (alebo mozno tam bude URL "natvrdo") a vrati obsah tej URL. Tj. uz bude zapuzdrena obsluha problemu se spojenim. Celkovo e.printStackTrace() znamena, ze nevis, co s tym. Mozno to vyhodit ven, nech sa to obsluzi vyssi? Mozno na to urobit vlastnu Exception classu? Toto sa da testovat ked namockujes alebo podstrcis metodu/tridu na downloadovani, co je tazsi.
  2. Dalsi dostane uz obsah a vytaha z neho data - v tvojem pripade skoro Book() bez tych dalsich metod. Pripadne aj vyhadze knihy, co nemaju zadny link (to moze byt aj dalsi krok). To uz ide otestovat, ci je to spravne, aj ked tam das 5 ruznych obsahu.
  3. Dalsi dostane knihy a cilovu zlozku a vrati mapu zdrojova URL na cilovy subor. To sa testuje v pohode.
  4. Dalsi dostane mapu ze zdroju na cil a vymaze take, co uz maju cilovy subor. Rovnako je testovani trivialne.
  5. Posledny stahne subory z urcenych URL v mape na urcene cile. Toto uz je s testovanim jak v bode 1.

Potom budes mnet nekde kod jak toto (zjednodusene a nazvy su trochu delsi pro nazornost; moje pevne stringy by mohli byt parametre funkcie alebo konstanty):

String rss = download("http://libri.../neco");
List<Book> allBooks = getAllBooksFromRSS(rss);
Map<String, String> allBooksUrlToFilename = getDownloadLocations(allBooks, "/home/peto/Download");
Map<String, String> notDownloadedBooksUrlToFilename = excludeExistingTargets(allBooksUrlToFilename);
downloadSourceDestination(notDownloadedBooksUrlToFilename);

Toto ma tu vyhodu, ze sa to cita dobre a nezanedbatelna cast sa da testovat lahko. Casti jak downloadSourceDestination a download s velku pravdepodobnostu uz nekdo naprogramoval tak, ze sa robia aj paralelne a efektivne. Alebo ich lahko urobis aj ty a nemaju moc spolecne s tvojim projektem - mozes ich pouzit aj inde. Mozno je to podobne aj pri excludeExistingTargets.

Na zamysleni: mozno ma byt getDownloadLocations mapa, kde je poradi naopak - subor je jak zdroj mapy. Nech sa da jedna URL stahnut aj na 2 rozlicne cile. Neupravoval sem to, ale je to asi este lepsi moznost.

Ano, poskytnuti treba downloadera sa vola dependency injection. Ale jak vidis, tak na test 3 z 5 metod (vlastne vsetkych s nejaku logiku) to vubec netreba - rovnako netreba ani mockovani.

volanie metod v konstruktori

Ano, to je casto zle - lebo mas este neskonstruovany objekt a volas v nom nieco. To nieco potom moze padnut, ked si na to nedas pozor.

V kazdem pripade mi pride, ze tie objekty musim v testovacej triede vytvorit nech uz ich predavam ako parameter do konstruktora alebo inak - ako som to robil doteraz.

Ano.

Musim to mockito este poskusat na jednoduchsich prikladoch lebo nerozumiem zatial ako to ma fungovat.

Mockito ti vrati objekt, u ktoreho nahradi jednu alebo viac metod. Ten objekt musis stale pouzivat. Neurobi "zazraky", ze by sa ten objekt automaticky dostal do tvojho vlastneho kontruktoru alebo podobne. Ked ho tam strcis, tak ano, ale to je tvoja akcia.