Make sure we treat tests differently

[sponsfreixes]

When using pytest fixtures, you are supposed to just declare them as input arguments, and they will be automagically loaded. For example:

import pytest
from myapp import meaning_of_life


@pytest.fixture
def foo():
    return 42

def test_meaning_of_life(foo):
    meaning = meaning_of_life()
    assert meaning == foo

This will violate rule 442, which is not desired in this case.

One could argue that fixtures should live on a different file than tests, but you run on the same issue if you have fixtures dependent on other fixtures:

import pytest


@pytest.fixture
def foo():
    return 42


@pytest.fixture
def bar(foo):
    return foo + 1

I think we need to make this feature "pytest aware", or just document that it there is conflict with it and might need to be disabled.

[sobolevn]

Yes, this is true.

I usually disable this check for all conftest.py files.
Could you please send a PR with the doc fix?

[sponsfreixes]

Will do!

[sobolevn]

@sponsfreixes are you still interested in fixing docs for this one? 🙂

[skarzi]

I think it would be really useful for new users to have somewhere in docs any example of per-file-ignores prepared specifically for tests and some comments with explanation, why we are suggest to ignore following violations in tests

[sobolevn]

@skarzi completely agree! I think even a whole new documentation entry under Usage might be added.

[sponsfreixes]

@sponsfreixes are you still interested in fixing docs for this one? 🙂

Whoa, it has been already a month since my last comment 😬

Yeah, I was still planning to do it. If somebody wants to steal it before I start, feel free to change assignee.

@skarzi what other violations should be ignored on tests?

[sobolevn]

Here several my projects that have a list of ignored violations for tests:

• https://github.com/sobolevn/django-split-settings/blob/master/setup.cfg#L36-L39
• https://github.com/dry-python/returns/blob/master/setup.cfg#L48
• https://github.com/wemake-services/dotenv-linter/blob/master/setup.cfg#L37-L38

Here's the ignore line for one private project:

  # Enable `assert` keyword and magic numbers for tests:
  tests/*.py: D103, S101, S105, WPS202, WPS210, WPS211, WPS336, WPS432

Basically I ignore:

1. Number of asserts, because I usually violate "one test - one assert" rule
2. I allow magic numbers more than often, because tests might have very strange logic on numbers in examples and test data
3. I allow to have a lot of module members inside a module (functions mostly)
4. Sometimes we have to allow a lot of arguments for test functions, because pytest passes fixtures as arguments
5. Sometimes it makes sense to have a lot of variables when you assemble some complex object from other fixtures / data

I can also ignore new violations from new linter versions in tests, because refactoring them is error-prone and has little value.

[skarzi]

My per-file-ignores for tests includes violations mentioned by @sobolevn and additionally following one:

• WPS118 - imho test functions/methods should have nice, meaningful names, so it's often better to use long but self-explanatory name for them