Ruff: lint pytest #4557

Avasam · 2024-08-11T22:52:51Z

Summary of changes

In this PR I've enabled https://docs.astral.sh/ruff/rules/#flake8-pytest-style-pt linting in Ruff and configured to the current preferences.

Pull Request Checklist

Changes have tests (these are test changes, all existing tests should pass)
News fragment added in newsfragments/. (no user facing changes)
(See documentation for details)

Avasam · 2024-08-11T22:54:42Z

ruff.toml

 ignore-fully-untyped = true

+[lint.flake8-pytest-style]
+parametrize-names-type = "csv"


I went with "csv" because it's the most used in the project currently. But I think I'd prefer the default of "tuple" See https://docs.astral.sh/ruff/rules/pytest-parametrize-names-wrong-type/ & https://docs.astral.sh/ruff/settings/#lint_flake8-pytest-style_parametrize-names-type

It should be OK to change to tuple if we have the linter to enforce it in future PRs.
Can ruff automatically fix that?
If I understand correctly the maintenance policy in the repository is that implicit config is better than explicit config, so it would be nice if we can rely on defaults.

It can autofix it, it's simply marked as an "unsafe fix". Happy to provide a follow-up PR or add it here (there's 41 hits)

Avasam · 2024-08-11T23:00:32Z

ruff.toml

 ignore = [
+	"PT007", # temporarily disabled, TODO: configure and standardize to preference
+	"PT011", # temporarily disabled, TODO: tighten expected error 
+	"PT012", # pytest-raises-with-multiple-statements, avoid extra dummy methods for a few lines, sometimes we explicitly assert in case of no error


I felt like the changes PT012 was asking for didn't fit the style of a handful of current tests, would've added more boilerplate without helping that much prevent false-negatives.

Avasam · 2024-08-12T12:56:18Z

Patch coverage failure because coverage is not aggregated (jaraco/skeleton#130) AND integration tests are not run when modifying integration tests. To be considered ?

Avasam · 2024-08-12T15:18:32Z

ruff.toml

 ]
 ignore = [
+	"PT007", # temporarily disabled, TODO: configure and standardize to preference
+	"PT011", # temporarily disabled, TODO: tighten expected error 


I think I'd like to see astral-sh/ruff#5157 or astral-sh/ruff#6840 before enabling PT011

Avasam · 2024-08-16T18:20:59Z

setuptools/tests/integration/test_pip_install_sdist.py

+    yield

-    request.addfinalizer(_debug_info)
+    # Let's provide the maximum amount of information possible in the case
+    # it is necessary to debug the tests directly from the CI logs.
+    print("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~")
+    print("Temporary directory:")
+    map(print, tmp_path.glob("*"))
+    print("Virtual environment:")
+    run([venv_python, "-m", "pip", "freeze"])


https://docs.astral.sh/ruff/rules/pytest-fixture-finalizer-callback/

Avasam · 2024-08-16T18:21:13Z

setuptools/tests/test_integration.py

+
+    yield cmd
+
+    # undo the monkeypatch, particularly needed under
+    # windows because of kept handle on cwd
+    monkeypatch.undo()
+    new_cwd.remove()
+    user_base.remove()
+    user_site.remove()
+    install_dir.remove()


https://docs.astral.sh/ruff/rules/pytest-fixture-finalizer-callback/

Avasam · 2024-08-21T02:33:22Z

setuptools/tests/test_setuptools.py

 @pytest.fixture(autouse=True)
 def isolated_dir(tmpdir_cwd):
-    yield
+    return


https://docs.astral.sh/ruff/rules/pytest-useless-yield-fixture/

abravalheri

Thank you very much.

abravalheri · 2024-10-31T12:55:59Z

ruff.toml

 ignore-fully-untyped = true

+[lint.flake8-pytest-style]
+parametrize-names-type = "csv"


It should be OK to change to tuple if we have the linter to enforce it in future PRs.
Can ruff automatically fix that?
If I understand correctly the maintenance policy in the repository is that implicit config is better than explicit config, so it would be nice if we can rely on defaults.

abravalheri · 2024-10-31T12:59:33Z

Patch coverage failure because coverage is not aggregated (jaraco/skeleton#130) AND integration tests are not run when modifying integration tests. To be considered ?

The integration tests are not included on purpose to reduce the pressure on the CI resources of the organization. For the aggregation, let's see what comes out of skeleton.

Avasam · 2024-10-31T14:21:59Z

Patch coverage failure because coverage is not aggregated (jaraco/skeleton#130) AND integration tests are not run when modifying integration tests. To be considered ?

The integration tests are not included on purpose to reduce the pressure on the CI resources of the organization.

Integration test was also excluded from coverage since I originally wrote that comment: #4589

abravalheri · 2024-11-04T18:14:45Z

Thank you very much!

Avasam commented Aug 11, 2024

View reviewed changes

Avasam force-pushed the Ruff--lint-pytest branch 2 times, most recently from d390533 to a72efef Compare August 11, 2024 23:26

Avasam commented Aug 12, 2024

View reviewed changes

Avasam commented Aug 16, 2024

View reviewed changes

Avasam force-pushed the Ruff--lint-pytest branch from 6cb42ab to 48a741d Compare August 21, 2024 02:32

Avasam commented Aug 21, 2024

View reviewed changes

Avasam force-pushed the Ruff--lint-pytest branch 4 times, most recently from d93e6e1 to 017a828 Compare August 27, 2024 15:57

Avasam force-pushed the Ruff--lint-pytest branch from 017a828 to bafdf65 Compare August 28, 2024 15:13

Avasam force-pushed the Ruff--lint-pytest branch from bafdf65 to 35873f1 Compare September 11, 2024 19:12

jaraco force-pushed the main branch from 809a9a7 to ffdf0bd Compare September 15, 2024 14:44

Avasam force-pushed the Ruff--lint-pytest branch from b83320b to 62b0419 Compare October 21, 2024 19:22

abravalheri approved these changes Oct 31, 2024

View reviewed changes

Avasam force-pushed the Ruff--lint-pytest branch from 59611f7 to baf6042 Compare October 31, 2024 14:25

abravalheri force-pushed the Ruff--lint-pytest branch from baf6042 to bf21018 Compare October 31, 2024 17:57

Avasam force-pushed the Ruff--lint-pytest branch from bf21018 to 14efd5f Compare November 1, 2024 18:17

Ruff: lint pytest

9fc7bbc

Avasam force-pushed the Ruff--lint-pytest branch from 14efd5f to 9fc7bbc Compare November 4, 2024 18:07

abravalheri merged commit b27393a into pypa:main Nov 4, 2024
17 of 18 checks passed

Avasam deleted the Ruff--lint-pytest branch November 4, 2024 18:15

Avasam mentioned this pull request Nov 4, 2024

Set Ruff's pytest's parametrize-names-type to default (tuple) #4730

Merged

2 tasks

Uh oh!

Ruff: lint pytest #4557

Ruff: lint pytest #4557

Uh oh!

Conversation

Avasam commented Aug 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Pull Request Checklist

Uh oh!

Avasam Aug 11, 2024

Choose a reason for hiding this comment

Uh oh!

abravalheri Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Avasam Aug 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Avasam commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Avasam Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Aug 16, 2024

Choose a reason for hiding this comment

Uh oh!

Avasam Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

abravalheri left a comment

Choose a reason for hiding this comment

Uh oh!

abravalheri Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

abravalheri commented Oct 31, 2024

Uh oh!

Avasam commented Oct 31, 2024

Uh oh!

Uh oh!

abravalheri commented Nov 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Avasam commented Aug 11, 2024 •

edited

Loading

Avasam Oct 31, 2024 •

edited

Loading

Avasam Aug 11, 2024 •

edited

Loading

Avasam commented Aug 12, 2024 •

edited

Loading