-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
PYTHON-2963 Add tox config in preparation for migration from setup.py #1240
Conversation
[tox] | ||
requires = | ||
tox>=4 | ||
envlist = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lack of specified Python versions here will cause tox to run using the default system interpreter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the plan is to migrate to tox in future PRs then could you update the title to something like "Add tox config"?
tox.ini
Outdated
commands = | ||
pre-commit run --all-files | ||
pre-commit run --all-files --hook-stage manual flake8 | ||
pre-commit run --all-files --hook-stage manual doc8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pre-commit run --all-files --hook-stage manual
will run all the pre-commit checks in one command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should update the github
workflow as part of this PR to use tox
.
Note that moving our evergreen tests to |
Sure, will do. |
For all of the workflow jobs? |
I think just the |
echo '{"strict": ["tests/test_typing_strict.py"]}' >> pyrightconfig.json | ||
pyright test/test_typing_strict.py | ||
|
||
[testenv:typecheck] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this environment, we can call the individual ones.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intended to replace needing an alias to run all of the typechecks in a single command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it is possible then to compose this to use the deps and commands of the other envs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Figured it out, fixed!
tox.ini
Outdated
{[testenv:typecheck-pyright]deps} | ||
allowlist_externals=echo | ||
commands = | ||
echo '{"strict": ["tests/test_typing_strict.py"]}' >> pyrightconfig.json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>
should be >
IIUC. Otherwise we'd repeatedly append to the same file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This change is still using
python setup.py test
inside tox to run the unit tests: migration to pytest will be done in https://jira.mongodb.org/browse/PYTHON-3727, and migration to pyproject.toml will be done in https://jira.mongodb.org/browse/PYTHON-2965.