Basic unit tests, logging, exception refactoring #25
Conversation
…ments file. Also updated setup.py to find namespace packages
…port folder so that all data from one session is in the same place
… custom args to be passed more easily in unit tests
…all, add basic logging, remove now-uncessary sys.exit()
… all windows and logging its exit status.
…emoved the timestamp as the directory now has a timestamp.
…eturn True if blink detected, False otherwise, and to check threshold > 0
|
tox doesn't want to start on travis for some reason! :( |
There was a problem hiding this comment.
After changing the first_namespace_packages by find_packages it executed tox, then failed with the same message as Travis.
------------------------------------------------------------------------------- live log call --------------------------------------------------------------------------------
2020-07-19 20:53:22 INFO Initiating tracking via Importer: cv
ERROR: InvocationError for command /home/kinow/Development/python/workspace/eyeloop/.tox/py38/bin/pytest --cov=eyeloop --cov-report html:tests/reports/coverage --html=tests/reports/pytest_results.html (exited with code 1)
___________________________________________________________________________________ summary ___________________________________________________________________________________
ERROR: py38: commands failed
Let me see if there's any easy flag to increase verbosity...
|
@cfculhane I think it needs an X Server to run the tests? I ran |
…the original find_packages
I've found, https://github.com/tox-dev/tox-travis , ill add that to the travis script to see what happens |
|
Still not working, I'll try this approach - jazzband/pip-tools#858 |
… if that resolves
|
Still no dice, open to ideas. Maybe it does need an X server and we could just run travis without a GUI - but given it was running eyeloop previously this doesn't seem to be the cause |
|
I'm not in front of the computer with my development environment, but I found this link from pytest-qt when searching for xfvb and tox. Check out the first error described, looks pretty similar to what we have here. https://pytest-qt.readthedocs.io/en/latest/troubleshooting.html Perhaps their solution could be ported to eyeloop? Or at least serve as starting point I think. |
Thanks for this, I'll look into it later in the week, I've unfortunately run out of time today |
|
Hallelujah, just using pytest works! Tox is still nice for easy of use by dev and end users I think, but running pytest seems to be the go for travis for now. |
|
Great work, guys! I will look into blink detection. Closing Importer via .release() is more elegant - I fully agree. Merging this into our main branch |
This PR combines a bunch of commits to address the following issues:
#8 - Implemented via shared_logging.py and its associated config file, at the moment the level is set to DEBUG which is quite verbose, but once the codebase becomes more stable this can be rolled back if desired
#22 - Using pytest and tox, I have written a basic integration test that runs two short videos and checks they are outputted correctly and that the output contains the expected number of frames. Later on further test can be written to test individual modules but at least now we can easily check if we break things as we change the code.
I have included calls to plugins to make coverage and test reports, see below:
#23 - partial fix to this, but the whole stack of .arm(), .release() etc might need to be looked at in more detail to figure out how untangle it and ensure things exit cleanly. For now I've removed the sys.exit() calls and made the exit clean by calling .release() when running out of frames. This might not be the intended behaviours, so will need feedback.
Please have a look through and see how I went, the blink_detection is a bit of an issue I think, I've added an assert that the blink_threshold > 0, as there is a check for abs(delta) > threshold if the threshold is less than 0, check_blink() will always detect a blink. #24 raises the issue of not hardcoding this, which might need to be addressed in a PR soon after this one.