tools to ensure consistency and proper test coverage about crazyflie-lib-python HOT 9 CLOSED

Hi, This all looks interesting but maybe we should take one at a time.

Pre-comit: Will that be a hard dependency? As I understood this will setup a hook to cleanup the code locally before committing it?

tox, coverage, pytest: My only question is, what is the relationship/difference between pytest and unittest we are using today? Otherwise these looks great and I would very gladly merge a pull request.

Makefile: My concern is that Makefiles does not work well on Windows. The way we have been approaching that today is to have python scripts in the tools folder.

from crazyflie-lib-python.

Pre-commit: This is a dev-dependency and will not be required for someone to run the package, however, it will ensure that people contributing to the package adhere to style guidelines because the hooks run right before the commit is allowed to be made. In some cases automatic changes are made for the user, in others the user may have to resolve the issues before they can commit their code. We have found this tool keeps us from wasting time and commenting on style in code reviews.

from crazyflie-lib-python.

pytest: One of the nice things about pytest is that it forces you to write tests in the form of functions. We have found that over time (in a very large code base), with our other framework Testify, people tend grab functionality that they shouldn't from classes, which results in a spiderweb of dependencies. We have stayed away from untittest because it is known to be buggy (dependability is important for a testing framework), some of its class based functionality, few choices for assertions, and lack of support between python versions.

tox: Invaluable for knowing that as you provide different packages for different versions of python, that you can be confident that you are not breaking anything.

coverage: We have caught many bugs with tests and code by being able to examine how much of the code is actually covered by tests.

from crazyflie-lib-python.

Makefile: My main concern is for the orchestration of all these tasks that we want to do with the package, such as creating a virtualenv to test the package, running the test suite, etc. These are actions that a user shouldn't have to remember every time. I can understand the lack of usage for windows, though typically I would use cygwin for windows development (I understand not everyone wants to add an extra dependency). I will take another look at the structure that you have setup, because since you are using docker, you are actually "able" to run linux type solutions on a windows pc. Maybe I can find something that works and isn't a nuisance for windows users.

from crazyflie-lib-python.

It seems like we were going back and forth in our office about whether unittest is now an effective substitute for py.test. Some arguments about unittest being buggy seem unsubstantiated and lack of support between python versions (namely 2.x and 3.x) doesn't seem to apply here. Maybe we dock the py.test discussion for now, since you already have a working unittest framework.

from crazyflie-lib-python.

That's a lot of info, Thanks! It all looks good.

For pre-commit, running things like pep/flake8 sounds good, actually modifying the code not so much. I like staying in control of what I am pushing and automatically changing codes still feel a bit dangerous.

For Makefile I think I do not mind but with the same reserve as for the rest: if it is a 'shortcut' that makes it easier in most situation it is good but not if it is mandatory to develop. Currently we run the python script tools/build/build in the CI to launch all required tools. Unfortunately it is not very pretty but is has the advantage to be portable on all platform the client is running on without extra dependencies.

If you do pull requests could you separate them per "theme"? It will make it easier to discuss there.

Thanks again
/Arnaud

from crazyflie-lib-python.

pre-commit can be run without auto-changing the code, although I would comment that we use this in our shop for a large python code base and don't have any issues with the auto re-writing functionality.

from crazyflie-lib-python.

I will start doing separate pull requests, like you suggested, and we can go from there.

from crazyflie-lib-python.

I will close this issue since we have started addressing these bit by bit.

from crazyflie-lib-python.