MarekSuchanek / labelord_tests

Tests for MI-PYT Labelord homework https://github.com/cvut/MI-PYT
Creative Commons Zero v1.0 Universal
3 stars 2 forks source link

false-pass výsledek testu pro úpravu štítku #12

Closed dragomirecky closed 6 years ago

dragomirecky commented 6 years ago

Test tests_web/test_hooks.py::test_label_edited uspěje i v případě, že nefunguje správně synchronizace editace štítku.

Problém je hlavně při špatném přejmenovávání štíku: pokud labelord dělá patch request na adresu s novým jménem štítku, request potichu failne (404). Test ale jediné co kontroluje je, že labelord udělá jeden patch request (což udělal), takže vše projde.

dragomirecky commented 6 years ago

Sorry, nechtěl jsem se přetahovat o správné jméno issue. Myslel jsem si, že jsem se předtím přepsal a tak jsem to znovu změnil na false-negative. Až pak jsem si všimnul, že jsi to změnil ty :). Každopádně, není false-negative náhodou správně? U testů se za pozitivní výsledek bere, když failne (jako v medicíně). Tenhle je negativní když nemá (nefailuje), takže false-negative, ne?

hroncok commented 6 years ago

je to asi jedno :D co třeba false-pass?

MarekSuchanek commented 6 years ago

Um... asi úplně nechápu.

Při patch request na adresu nového štítku namísto starého dostanu:

betamax.exceptions.BetamaxError: A request was made that could not be handled.

A request was made to https://api.github.com/repos/MarekSuchanek/repocribro/labels/RFE that could not be found in test_hooks.test_label_edited.

The settings on the cassette are:

    - record_mode: none
    - match_options {'mipyt-github', 'method', 'uri'}.
dragomirecky commented 6 years ago

Jo aha, ty necháš dopropagovat tu exception pri zmene labelu až ven z flask handleru (a tedy ti ten webhook request „z githubu“ neuspeje). Já ji odchytnu a pokracuju v synchronizaci dalsiho repozitare.

Urcite by u me v kodu bylo lepsi odchytavat jenom urcite vyjimky, ktere vim, ze mohou nastat (ted tam humpolácky odchytávám všechny, tedy i BetamaxError).

MarekSuchanek commented 6 years ago

Takže bych měl přidat nějaký test, kde ta kazeta bude schválně chybět, abychom zjistili, že se ta výjimka nechytá v kódu 😄 Ideálně pro každou operaci zvlášť, kdyby to náhodou někdo chytal už takhle "blízko".

Nebo ve všech odevzdaných úlohách vyhledat except a:

# original try block
except BetamaxError as e:
    raise e
# original except block

A nebo máš @hroncok prosím jiný nápad?

hroncok commented 6 years ago

Monkeypatchovat BetamaxError na spy a v teardownu počítat, kolikrát se raisnul? (Řečeno hezky česky.)

hroncok commented 6 years ago

(Zavřeno omylem.)

MarekSuchanek commented 6 years ago

OK, díky. Zítra odpoledne si s tím pohraju, bohužel se k tomu nedostanu kvůli výuce dřív...

Zkusím 🐒patch + spy a když s tím bude víc problémů než užitku, tak přidám jednoduše tři testy bez kazety (nebo se špatnou kazetou 😈) s pytest.raises.

MarekSuchanek commented 6 years ago

@hroncok myslel si nějak takhle #14? Vypadá to, že to funguje... ale nějak podezřele jednoduché :smile: