astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.63k stars 1.09k forks source link

Add config to disable S101 (assert detected) and a few others in test folders #4368

Open jankatins opened 1 year ago

jankatins commented 1 year ago

Per convention, testfolders usually reside in <reporoot>/test/ or <reporoot>/tests/. Right now, every time I add certain rules, specifically S101, ARG, FBT, and a few others (e.g. via "ALL"), I have to add a folder exclude for these rules for the tests/ folder:


[tool.ruff.per-file-ignores]
"tests/**/*.py" = [
    # at least this three should be fine in tests:
    "S101", # asserts allowed in tests...
    "ARG", # Unused function args -> fixtures nevertheless are functionally relevant...
    "FBT", # Don't care about booleans as positional arguments in tests, e.g. via @pytest.mark.parametrize()
    # The below are debateable
    "PLR2004", # Magic value used in comparison, ...
    "S311", # Standard pseudo-random generators are not suitable for cryptographic purposes
]

It would be really nice if ruff would do that automatically, E.g. by disabling the above rules via a central test-files list which is on a commonly used default:

[tool.ruff]
# To disable tests which are ok in tests -> would not need to be set as it would be a nice default
test-files =  ["tests/**/*.py", "test/**/*.py"]

This was already discussed in https://github.com/charliermarsh/ruff/issues/714 where the original reporter (@harupy) closed the issue after confirming the per folder overwrites.

jpmckinney commented 1 year ago

FWIW, I ignore the same rules (including the debatable ones), plus "D".

paduszyk commented 3 months ago

@jankatins I totally agree with your idea 👍🏻 I would also propose to add S105, S106, and S107 to your list.

Hardcoded passwords usually appear in tests of authentication stuff. However, I am not entirely sure. For instance, this would require assuming that testers use naive "placeholder" passwords, not real passwords (maybe this could be set up via an extra flake8-bandit setting)...

@jankatins @jpmckinney What do you think?

jpmckinney commented 3 months ago

Sounds good to me. I haven't had those rules trigger, even though I do have hardcoded passwords in tests, but I would exclude them if they were triggering.

charliermarsh commented 3 months ago

Are there some opportunities to improve our rules here? E.g., avoid flagging unused arguments in pytest fixtures, if we can detect them?

tomsquest commented 3 months ago

For the one finding that issue, as of Ruff 0.5.4, I adapted @jankatins snippet to:

[tool.ruff.lint.extend-per-file-ignores]
"tests/**/*.py" = [
    # at least this three should be fine in tests:
    "S101", # asserts allowed in tests...
    "ARG", # Unused function args -> fixtures nevertheless are functionally relevant...
    "FBT", # Don't care about booleans as positional arguments in tests, e.g. via @pytest.mark.parametrize()
    # The below are debateable
    "PLR2004", # Magic value used in comparison, ...
    "S311", # Standard pseudo-random generators are not suitable for cryptographic purposes
]