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

[chore] Remove black and ruff from test-requirements #18139

Merged
merged 3 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
exclude: '^(mypyc/external/)|(mypy/typeshed/)|misc/typeshed_patches' # Exclude all vendored code from lints
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0 # must match test-requirements.txt
rev: v4.5.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- repo: https://github.com/psf/black-pre-commit-mirror
rev: 24.8.0 # must match test-requirements.txt
rev: 24.8.0
hooks:
- id: black
exclude: '^(test-data/)'
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.6.9 # must match test-requirements.txt
rev: v0.6.9
hooks:
- id: ruff
args: [--exit-non-zero-on-fix]
Expand Down
10 changes: 8 additions & 2 deletions test-data/unit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ First install any additional dependencies needed for testing:

python3 -m pip install -U -r test-requirements.txt

Configure `pre-commit` to run the linters automatically when you commit:

pre-commit install

The unit test suites are driven by the `pytest` framework. To run all mypy tests,
run `pytest` in the mypy repository:

Expand Down Expand Up @@ -157,9 +161,11 @@ To run mypy on itself:

python3 -m mypy --config-file mypy_self_check.ini -p mypy

To run the linter:
To run the linter (this commands just wraps `pre-commit`, so you can also
invoke it directly like `pre-commit run -a`, and this will also run when you
`git commit` if enabled):

ruff .
python3 runtests.py lint
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It reads a bit odd when this command uses runtests.py, and the very next line says "You can also run all of the above tests using runtests.py". It may be good to mention the underlying lint command here. FWIW, I always run pre-commit directly, and only just learned that the runtests.py invocation exists.

Suggested change
python3 runtests.py lint
pre-commit run -a

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to run lint using runtests.py, since it lets me perform several tasks via a single command, such as python3 runtests.py lint self pytest-fast. If we will remove black and ruff from test requirements, we should probably change runtests.py lint to run pre-commit run -a in order to avoid breaking existing workflows.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably change runtests.py lint to run pre-commit run -a in order to avoid breaking existing workflows.

It looks like that's already the case :-)

mypy/runtests.py

Lines 68 to 69 in 54a2c6d

# Lint
"lint": ["pre-commit", "run", "--all-files"],

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, forgot about it!

Copy link
Contributor Author

@sterliakov sterliakov Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to still mention python runtests.py lint here since it's difficult to discover by other means. I expanded the preceding paragraph and added pre-commit install mention - does this look better now?

(I also use pre-commit pretty much everywhere so prefer to invoke it directly)


You can also run all of the above tests using `runtests.py` (this includes
type checking mypy and linting):
Expand Down
2 changes: 0 additions & 2 deletions test-requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
-r mypy-requirements.txt
-r build-requirements.txt
attrs>=18.0
black==24.8.0 # must match version in .pre-commit-config.yaml
filelock>=3.3.0
# lxml 4.9.3 switched to manylinux_2_28, the wheel builder still uses manylinux2014
lxml>=4.9.1,<4.9.3; (python_version<'3.11' or sys_platform!='win32') and python_version<'3.12'
psutil>=4.0
pytest>=8.1.0
pytest-xdist>=1.34.0
pytest-cov>=2.10.0
ruff==0.6.9 # must match version in .pre-commit-config.yaml
setuptools>=75.1.0
tomli>=1.1.0 # needed even on py311+ so the self check passes with --python-version 3.8
pre_commit>=3.5.0
20 changes: 3 additions & 17 deletions test-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@
#
attrs==24.2.0
# via -r test-requirements.in
black==24.8.0
# via -r test-requirements.in
cfgv==3.4.0
# via pre-commit
click==8.1.7
# via black
coverage==7.6.1
# via pytest-cov
distlib==0.3.9
Expand All @@ -29,21 +25,13 @@ iniconfig==2.0.0
lxml==4.9.2 ; (python_version < "3.11" or sys_platform != "win32") and python_version < "3.12"
# via -r test-requirements.in
mypy-extensions==1.0.0
# via
# -r mypy-requirements.txt
# black
# via -r mypy-requirements.txt
nodeenv==1.9.1
# via pre-commit
packaging==24.1
# via
# black
# pytest
pathspec==0.12.1
# via black
# via pytest
platformdirs==4.3.6
# via
# black
# virtualenv
# via virtualenv
pluggy==1.5.0
# via pytest
pre-commit==3.5.0
Expand All @@ -61,8 +49,6 @@ pytest-xdist==3.6.1
# via -r test-requirements.in
pyyaml==6.0.2
# via pre-commit
ruff==0.6.9
# via -r test-requirements.in
tomli==2.0.2
# via -r test-requirements.in
types-psutil==6.0.0.20241011
Expand Down
Loading