[flake8-pytest-style] Test function parameters with default arguments (PT028)

[InSyncWithFoo]

Summary

Resolves #15412.

Test Plan

cargo nextest run and cargo insta test.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+95 -0 violations, +0 -0 fixes in 6 projects; 49 projects unchanged)

PlasmaPy/PlasmaPy (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ tests/formulary/test_thermal_speed.py:400:43: PT028 Test function parameter `kwargs` has default argument

apache/airflow (+49 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ helm_tests/airflow_aux/test_container_lifecycle.py:102:54: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_container_lifecycle.py:123:59: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_container_lifecycle.py:168:65: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_container_lifecycle.py:187:64: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_container_lifecycle.py:208:68: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_container_lifecycle.py:41:52: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_container_lifecycle.py:78:45: PT028 Test function parameter `hook_type` has default argument
+ helm_tests/airflow_aux/test_pod_template_file.py:927:84: PT028 Test function parameter `hook_type` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:1029:66: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:243:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:253:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:263:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:332:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:350:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:362:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:373:58: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:402:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:412:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:445:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:473:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:640:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:654:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:686:60: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:809:66: PT028 Test function parameter `initial_batch_size` has default argument
+ providers/tests/amazon/aws/hooks/test_eks.py:819:66: PT028 Test function parameter `initial_batch_size` has default argument
... 24 additional changes omitted for project

apache/superset (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ superset/cli/test_db.py:141:66: PT028 Test function parameter `raw_engine_kwargs` has default argument
+ tests/integration_tests/db_engine_specs/base_engine_spec_tests.py:50:63: PT028 Test function parameter `engine_spec_class` has default argument

pandas-dev/pandas (+4 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ asv_bench/benchmarks/gil.py:39:31: PT028 Test function parameter `num_threads` has default argument
+ asv_bench/benchmarks/gil.py:39:46: PT028 Test function parameter `kwargs_list` has default argument
+ pandas/util/_tester.py:15:41: PT028 Test function parameter `extra_args` has default argument
+ pandas/util/_tester.py:15:68: PT028 Test function parameter `run_doctests` has default argument

zulip/zulip (+4 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ tools/lib/test_server.py:56:34: PT028 Test function parameter `skip_provision_check` has default argument
+ tools/lib/test_server.py:57:26: PT028 Test function parameter `external_host` has default argument
+ tools/lib/test_server.py:58:28: PT028 Test function parameter `log_file` has default argument
+ tools/lib/test_server.py:59:18: PT028 Test function parameter `dots` has default argument

astropy/astropy (+35 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ astropy/io/votable/tests/test_vo.py:950:36: PT028 Test function parameter `test_path_object` has default argument
+ astropy/modeling/tests/test_models.py:47:41: PT028 Test function parameter `amplitude` has default argument
+ astropy/modeling/tests/test_models.py:47:54: PT028 Test function parameter `frequency` has default argument
+ astropy/stats/tests/test_bayesian_blocks.py:10:36: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_bayesian_blocks.py:167:35: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_bayesian_blocks.py:20:33: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_bayesian_blocks.py:35:47: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_histogram.py:101:38: PT028 Test function parameter `N` has default argument
+ astropy/stats/tests/test_histogram.py:101:50: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_histogram.py:112:43: PT028 Test function parameter `N` has default argument
+ astropy/stats/tests/test_histogram.py:112:55: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_histogram.py:172:30: PT028 Test function parameter `N` has default argument
+ astropy/stats/tests/test_histogram.py:172:42: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_histogram.py:17:28: PT028 Test function parameter `N` has default argument
+ astropy/stats/tests/test_histogram.py:17:41: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_histogram.py:31:31: PT028 Test function parameter `N` has default argument
+ astropy/stats/tests/test_histogram.py:31:44: PT028 Test function parameter `rseed` has default argument
+ astropy/stats/tests/test_histogram.py:60:28: PT028 Test function parameter `N` has default argument
... 17 additional changes omitted for project

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
PT028	95	95	0	0	0

[InSyncWithFoo]

is_likely_pytest_test() and is_likely_pytest_test_file() were taken almost verbatim from #15158. The only change is that the former now no longer calls the latter. This is because flake8-pytest-style says a function is a test function when its name starts with test_.

Aside from that change, I think is_likely_pytest_test()'s divergence is for the better. I can't say the same about is_likely_pytest_test_file(), however, so it is currently dead code. What should be done with these two?

[MichaReiser]

Thank you. I've a few comments regarding the implementation and I think we should remove the fix.

[MichaReiser]

Did you review the ecosystem changes?

[InSyncWithFoo]

@MichaReiser Yes. There are a couple of false positives. For example:

@pandas-dev/pandas:

def test_parallel(num_threads=2, kwargs_list=None):
    """
    Decorator to run the same function multiple times in parallel.
    ...
    """
    ...

@zulip/zulip:

def test_server_running(
    skip_provision_check: bool = False,
    external_host: str = "testserver",
    log_file: str | None = None,
    dots: bool = False,
) -> Iterator[None]:
    ...

These kinds of false positives can be reduced (the former would no longer be reported) by making is_likely_pytest_test() call is_likely_pytest_test_file(). That would be quite a divergence from upstream, however, as discussed above.

[MichaReiser]

Thanks for addressing the feedback.

I'm leaning towards not adding the is this a pytest file just yet because it would introduce the need for a new option for those not using the default file name patterns. We can decide to restrict the rule in the future if this comes up often. The other reason why I'm for deferring is that there are still some false negatives because many of the non-test test functions are defined in files prefixed with test_