Configure tox by EliahKagan · Pull Request #1667 · gitpython-developers/GitPython (original) (raw)

This adds a tox.ini file so that tox can be run locally to test on 3.7, 3.8, 3.9, 3.10, 3.11, and 3.12, or whichever of them are available, and also run the lint, formatting, and style checks.

Because CI also tests building the documentation, and it is convenient to be able to use tox to check all the things CI checks (albeit on just one's local platform), I have added a tox environment for that too. But because it actually writes to the build directory, I have not listed it in env_list at the top, so it doesn't run automatically--it can still be run with tox -e html. That could be improved in the future, but I think reasonable ways of doing so would involve modifying doc/Makefile to make it more configurable, and that seems outside the proper scope of this PR.

This PR does not use tox on CI. It is possible to use tox to drive tests on CI, such as with the tox-gh-actions plugin, but this does not do that, because:

• I feel it would be outside the scope of this PR, because the benefit of tox for this project is not primarily for what it brings to CI nor even for making local and CI based testing more consistent.
• If it is to be done, I think it may be better to do it after native Windows tests are added, which may have bearing on how (and maybe even if) it should be done.
• Most importantly, migrating from a more traditional GitHub Actions workflow for testing to a tox workflow will sometimes make results harder to read, because they are no longer divided into top-level steps belonging to the test job and clearly shown as separate. This doesn't mean it shouldn't be done, and either manual customization or alternatives to tox-gh-actions such as tox-gh-matrix might help.
• In my view, the biggest gains from using tox on CI are for when the tox environments are more granular than the CI jobs. This happens in projects that need to parameterize their test runs not just on OS and Python version but also on dependency versions (while avoiding the proliferation of separate CI jobs), which I don't think is applicable here.

So while I suspect it may eventually be a good idea to use tox on CI, I don't think it should be done until it is clearly no worse than the current approach and other readily available approaches.

My intention is also not for tox to replace other ways of running tests when developing locally. During routine development, tox may be too slow or its output too complicated, for regularly checking the status of some or all tests. Furthermore, pre-commit seems to be working great for the lint checks--and the lint tox environment I configured uses it, to as to pick up any new or changed linting tools automatically. (Also, not everyone likes tox.)

For these reasons, I have not modified the readme to replace anything with tox-based instructions. However, the readme could be modified to also talk about tox. I am not sure if that should be done either, because the users most interested in using tox are those already familiar with it, and they will notice the tox.ini. Furthermore, I think it may be best to see how it works out (for example, should it really have the environment for testing that we can build docs by actually building them?) before adding it to the readme or other project documentation. However, if you'd like tox instructions, or a mention of it, etc., added the readme, I'd be pleased to add it (as well as to make other requested changes).

Some other considerations:

• The one problem I encountered initially in using tox to run tests was being prompted to enter my passphrase for some tests. Because that completely breaks the useful automation tox provides, I allowed tox to pass environment variables whose names start with SSH_ through to its test environments. It would probably be sufficient to pass through SSH_AGENT_PID and SSH_AUTH_SOCK; I'd be pleased to change it to just that, if requested. (If other forms of authentication than SSH that I'm not using rely on environment variables being set, then that might still be broken.)
• Because the version of Python being tested on CI for Cygwin is 3.9, which is also the latest version available through the Cygwin package manager, the tox environments for everything other than the tests are specified as 3.9, to support Cygwin and to allow results to be compared between Cygwin and other platforms. Also, because Cygwin doesn't have 3.10, 3.11, or 3.12, I have not specified skip_missing_interpreters = true, leaving it to the default of false. Nonetheless, I do not claim tox behaves perfectly on Cygwin in this configuration. I sometimes have problems where it tries to use Python interpreters from my native system before eventually finding something it can really use.
• I've removed tox from requirements-dev.txt, and not added it to test-requirements.txt since it should rarely be installed together with other project dependencies in the same environment. It's possible to install tox in a virtual environment that one is using for the project, but this is not typically done, because tox creates its own environments. It's best to install tox with pipx when possible. Furthermore, if any version of tox is available, and that version is lower than the version specified in tox.ini, then tox will bootstrap a virtual environment and install a newer version of itself in it. Of course, one can still run pip install tox in a virtual environment (or the global environment) if one wishes to get tox that way.
• I considered putting the tox configuration in pyproject.toml to avoid adding yet another top-level file. But I decided against it. It forgoes the discoverability benefits noted above in my reasoning about why I haven't mentioned tox in the readme. Also, it has to be given as one giant string, and editors (and GitHub) don't currently special-case that with useful syntax highlighting.