Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove tox #488

Open
PerchunPak opened this issue Feb 18, 2023 · 3 comments
Open

Remove tox #488

PerchunPak opened this issue Feb 18, 2023 · 3 comments
Assignees
Labels
area: CI Related to continuous intergration and deployment

Comments

@PerchunPak
Copy link
Member

If we googled "why to use tox", there will be a list of reasons. But those are useless in present days, mostly because of CI and poetry existence.

  1. Tox aims to fix incompatibility between machines.
    This is what poetry already does! It installs own venv for every project and ensures that all requirements (like python version or packages versions) were met.
  2. Tox simplifies setup system.
    Tox complicates setup system. Instead of just having poetry install && pytest, we need to use a bunch of workarounds and tools. Like tox-poetry, poetry inside our dependencies (see this discussion) etc.
  3. Tox makes it easy to test against different versions of Python.
    This had lost its actuality when Python 2 was deprecated. Python is too backwards compatible, so there is no need to run tests on dev machine. Instead, CI exists, which pretty rarely fails by this reason.

There also were ~10 more reasons, why to use tox, but the answer on all of those was just "use CI/poetry, it covers such case much better".


Although, tox is still used in releasing process, to ensure that exact release works. We can just replace it with automatic releases, as it's done in mcproto.

@ItsDrike
Copy link
Member

ItsDrike commented Feb 18, 2023

We also use tox for some other tasks, such as building the docs, since similarly to a makefile, tox allows subcommands to do different things.

We can solve this by moving to something like taskipy, acting sort of similarly to simple shell aliases. As an example of how can look, see mcproto's pyproject.toml.

The issue with being able to run tests for multiple versions locally is indeed a bit more annoying to solve, however as @PerchunPak mentioned, it's not all that necessary. That said, there is actually a way to handle local testing too, here's how:

  • Install python 3.8 on your machine (ideally with pyenv: pyenv install 3.8.13
  • Run poetry env use 3.8, which will create a 3.8 environment for the project, alongside the original
    • Note: if you have poetry configured to create environments in projects, this will overwrite that environment instead, and I'm not sure if there's any good way to avoid it, as there's no way to specify the resulting venv path with this command. (Could be worth creating an issue to poetry about)
  • Install the dependencies: poetry install
  • Run pytest: poetry run pytest
  • Go back to your system python version: poetry env use system
    • This should just switch you back to the original environment, without creating a new one again, that is, unless you're using the in-project environments.

Sure, this is much more annoying than tox, but the fact that it's still doable at all is nice, and considering there's usually no need to actually do this, it might be sufficient. (See more about this in poetry docs)

Although, tox is still used in releasing process, to ensure that exact release works. We can just replace it with automatic releases, as it's py-mine/mcproto#7.

How is tox used in the release proecss? All I see is poetry run tox --recreate command in release.sh, which just runs all of the tests and linters before a release, as a sort of sanity check. Without tox, we'd have to run these on the used python version alone instead, but if we do so only after seeing that CI tests were successful, this should still be ok. It's not like tox is hugely integrated into our release proecess.

@PerchunPak
Copy link
Member Author

Note: if you have poetry configured to create environments in projects, this will overwrite that environment instead, and I'm not sure if there's any good way to avoid it, as there's no way to specify the resulting venv path with this command. (Could be worth creating an issue to poetry about)

I think it's expected behavior and the reason, why poetry by default do not create venvs in project folder.

How is tox used in the release proecss? All I see is poetry run tox --recreate command in release.sh, which just runs all of the tests and linters before a release, as a sort of sanity check.

Yes, I meant exactly this part. It can be easily replaced by CI in automatic release system.

It's not like tox is hugely integrated into our release proecess.

Just, I asked before in Discord "why do we still need tox?" and the answer was "for testing releases".

@kevinkjt2000
Copy link
Contributor

Automatically creating releases from the pipeline would alleviate the need for a local release.sh script.

@kevinkjt2000 kevinkjt2000 added the area: CI Related to continuous intergration and deployment label Feb 20, 2023
@kevinkjt2000 kevinkjt2000 self-assigned this Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: CI Related to continuous intergration and deployment
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants