Rule request

Description

Right now this is not detected as an error:

import pytest

pytest.skip('Ignore this module')

Rationale

This is also an error with pytest:

_____________ ERROR collecting tests/test_multiprocess_aioredis.py _____________
Using pytest.skip outside of a test will skip the entire module. If that's your intention, pass `allow_module_level=True`. If you want to skip a specific test or an entire class, use the @pytest.mark.skip or @pytest.mark.skipif decorators.

Today I got hit by a xfail problem. My module was decorated with pytest.mark.xfail, because I wanted to test an explicit test failure in my plugin: https://github.com/dry-python/returns/blob/2223f03a2dbb96d4cd273fb41ad5c26331d9eef8/tests/test_contrib/test_hypothesis/test_laws/test_wrong_custom_type_with_init.py#L10

But, under some condition, I've noticed that sometimes it was failing due to an unknown reason.
It was really hard to find, because this test was passing for a year now.

So, my idea is to:

• Allow pytest.mark.xfailure(raises=ExceptionType)
• Forbid pytest.mark.xfailure or ``pytest.mark.xfailure()`

Docs: https://docs.pytest.org/en/6.2.x/skipping.html#raises-parameter

It hides errors 😞

@contextmanager
def cli():
    interface = CLI()
    try:
        yield interface
    except subprocess.CalledProcessError as cpe:
        traceback.print_exc()
        interface.warning(f"stdout: {cpe.stdout}")
        interface.error(f"stderr: {cpe.stderr}")
        sys.exit(1)

This context manager was built to provide a commandline interface. Since during the execution of the rest of the code, a number of subprocess calls happen and, well, there's always a chance of unexpected errors there, I placed this try/except in there. This ensures that in case of unexpected error, as much info as possible is printed.

However, I have found that I cannot test this with just a "simple statement". The most basic test I can run is this:

def test_cli(capsys):
    with pytest.raises(SystemExit):
        with library.cli():
            raise CalledProcessError(1, "cmd", output="findme", stderr="findmetoo")

    stdout, stderr = capsys.readouterr()
    assert stdout.splitlines() == ["stdout: findme", "stderr: findmetoo"]

Now, I did read #93 so I am aware of the possible work arounds.

But I thought I might file this bug report in case you do see a simpler way of testing it, or if maybe you saw a way to add it as an exception.

Rule request

Description

Allow assert statement in pytest.raises() block. Rule – PT012.
Now it is impossible to check the correctness of the Exception:

Example:

with pytest.raises(CustomError) as exc_info:
    client.send_sms(sms)

    assert exc_info.value.http_status == expected_status
    assert exc_info.value.description == expected_description

Error: PT012 pytest.raises() block should contain a single simple statement

Rationale

There is no alternatives to check Exceptions correctness.

Rule request

I want a rule that prevents users from writing potentially incorrect tests.

Thesis

This is what I have in mind.

Correct use:

def test_xxx():
    tested_thing = do_some_init()
    with pytest.raises(SomeError):
        tested_thing.some_meth()  # Should raise SomeError

Potentially problematic use:

def test_xxx():
    with pytest.raises(SomeError):
        tested_thing = do_some_init()  # <- even if the test worked properly when first comitted, there's a chance that this line will start raising SomeError at some point in the future making the next one unreachable, thus turning this test into an incorrect one
        tested_thing.some_meth()  # Should raise SomeError

Reasoning

This would prevent silly mistakes in tests.

Original issue is opened by @webknjaz here: wemake-services/wemake-python-styleguide#1110

Rule request

Description

pytest allows to use marks both with parentheses and without it, for example:

@pytest.mark.asyncio
async def test_example1() -> None:
    ...


@pytest.mark.asyncio()
async def test_example2() -> None:
    ...

Rationale

It causes inconsistency. This rule can be generalized to any marks, but at least I care about asyncio mark.

Rule request

Description

Rule should check the order of main elements in a test file, e.g. fixtures first, then test classes/functions, then helpers. Rule should be configurable.

Rationale

Inspired by eslint-plugin-jest rule prefer-hooks-on-top.

Bad:

def test_foo(_fixture):
    pass

Good:

@pytest.mark.usefixtures('_fixture')
def test_foo():
    pass

Rationale: value of _fixture is always None per PT004/PT005

Bug report

What's wrong

@pytest.fixture()
def _any_fixture(mocker):  # PT005 fixture '_any_fixture' returns a value, remove leading underscore
    def nested_function():
        return 1

    mocker.patch('...', nested_function)

How it should work

PT005 error should not happen.

System information

• Operating system: macOS
• Python version: 3.8
• flake8 version: 3.8.3
• flake8-pytest-style version: 0.1.3

Bug report

What's wrong

I use the following pattern in a few places, where a base class defines tests, some of which expect fixtures to be defined in child classes.

from abc import ABC, abstractmethod
import pytest


class BaseTest(ABC):
    @pytest.fixture(scope="class")
    @abstractmethod
    def my_value(self):
        raise NotImplementedError

    def test_value_even(self, my_value):
        assert my_value % 2 == 0


class TestImplementation(BaseTest):
    @pytest.fixture(scope="class")
    def my_value(self):
        return 2

This leads to the error:

test.py:8:5: PT004 fixture 'my_value' does not return anything, add leading underscore

There are a few ways to avoid this, but none are totally satisfactory.

1. Remove the fixture decorator. This would require a comment mentioning that it's supposed to be a fixture, and any special arguments (e.g. scope) to go with it.
2. Remove the abstract method decorator. This makes you lose the safety it gives, you would be allowed to instantiate classes which don't have it defined.
3. Return a value in the abstract method. It is still possible to call the implementation of abstract methods, this might cause unexpected behavior. You can return a value after an unconditional raise NotImplementedError to avoid this, but that leads to unreachable code errors.
4. Move the fixture code into setup_class. Not all fixtures can be moved, they may depend on other fixtures, and they may return values which can't just be set in a global.
5. Use a noqa. Does this one even need an explanation?

How it should work

Ideally anything decorated with abstractmethod should be ignored for PT004, and possibly also PT005.

System information

> flake8 --bug-report
{
  "dependencies": [],
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.9.0",
    "system": "Windows"
  },
  "plugins": [
    {
      "is_local": false,
      "plugin": "flake8-pytest-style",
      "version": "1.3.0"
    },
    {
      "is_local": false,
      "plugin": "mccabe",
      "version": "0.6.1"
    },
    {
      "is_local": false,
      "plugin": "pycodestyle",
      "version": "2.6.0"
    },
    {
      "is_local": false,
      "plugin": "pyflakes",
      "version": "2.2.0"
    }
  ],
  "version": "3.8.4"
}

See pytest-dev/pytest-mock#182

PT008 should recognize the new fixtures and produce the same warnings as with mocker.

• Prefer pytest.fail() to assert 0 / assert False
• Always pass a message to pytest.fail()

Currently we use both flake8-pytest and flake8-pytest-style

I would love to drop flake8-pytest dependency, because:

• It fails on windows, see wemake-services/wemake-python-styleguide#337
• There are no issue to report on Github
• This project is not active for the last 3 years

We can port the only violation there is from flake8-pytest, its source code is tiny: https://github.com/vikingco/flake8-pytest/blob/master/flake8_pytest.py

And after that there would be no need in this extra dependency. flake8-pytest-style would be enough. By the way, thank you for an awesome library!

Rule request

Description

Disallow pytest.yield_fixture in favor of pytest.fixture

Rationale

yield_fixture is a deprecated alias for fixture since pytest 3.0, which was released in 2016.

This code does not produce any warnings:

from pytest import fixture

@fixture
def test_some():
    assert 1

But, it should! @fixture use is invalid.

I can provide false-negative examples about each specific warning there is. And it all happens due to the fact that you allow to import things from pytest with from keyword.

The only possible way to import pytest should be defined as import pytest
This way we can at least partially be sure about what is going on.

Today I thought that eslint-plugin-jest can be a great inspiration for this project.
It is a well established project that does the same thing but for eslint and jest test runner in JavaScript.

It is possible that they also enforce some best practices that this project does not yet.

https://github.com/jest-community/eslint-plugin-jest

Rule request

Description

We only need fixtures like this one:

@pytest.fixture
def some_fixture():
    yield 1
    print('Teardown')

When there's actually a teardown process. I sometimes see a code like this:

@pytest.fixture
def some_fixture():
    yield 1

Or:

@pytest.fixture
def some_fixture():
    print('Setup')
    yield 1

So, basically I suggest to forbid that yield statement is the last one in a function's body.

Rationale

It does not make sence. And semantically incorrect. Use return 1 instead.

Bug report

What's wrong

        pytest.fail(reason=msg)

causes PT016 no message passed to pytest.fail().

Does it not like that it's a keyword arg or that the value is a var and not a string literal?

How it should work

No error.

System information

N/A

Bad:

assert something and something_else

Good:

assert something
assert something_else

Rationale: simplify assertions, check each result separately

Bug report

re-disallow addfinalizer in factories as fixtures pattern:

What's wrong

I have

# "Factories as fixtures" pattern
@pytest.fixture()
def my_factory(request):
    def create_resource(arg):
        resource = acquire_resource(arg)
        request.addfinalizer(resource.release)
        return resource
    return create_resource

How it should work

it should report the addfinalizer call in favour of yield fixtures:

# "Factories as fixtures" pattern
@pytest.fixture()
def my_factory(request):
    with contextlib.ExitStack() as stack:
        def create_resource(arg):
            resource = acquire_resource(arg)
            stack.callback(resource.release)
            return resource

        yield create_resource

or even better:

@contextlib.contextmanager
def _cm_resource(arg):
    resource = acquire_resource(arg)
    try:
        yield resource
    finally:
        resource.release()

# "Factories as fixtures" pattern
@pytest.fixture()
def my_factory(request):
    with contextlib.ExitStack() as stack:
        def create_resource(arg):
            return stack.enter_context(_cm_resource(arg))

        yield create_resource

System information

• Operating system: NA
• Python version: NA
• flake8 version: NA
• flake8-pytest-style version: NA

• Python version: 3.6
• flake8-pytest-style version used: 0.6.0

Description

Running flake8 I'm getting PT006 for a piece of code that I think it's right. Can you explain why is this happening?

What I Did

The code:

@pytest.mark.parametrize("date_range, expected", [
    (("2019-01-20", "2019-01-20"), ["2019-01-20"]),
    (("2019-01-20", "2019-01-21"), ["2019-01-20", "2019-01-21"]),
    (("2019-01-20", "2019-01-22"), ["2019-01-20", "2019-01-21", "2019-01-22"]),
    (("2020-02-28", "2020-03-01"), ["2020-02-28", "2020-02-29", "2020-03-01"])
])
def test_get_date_range(date_range, expected):
    start, end = date_range

    assert get_date_range(start, end) == expected

The configuration:

[flake8]
max-line-length = 120

The execution:

pipenv run flake8
Loading .env environment variables…
artifacts/src/test_date_range.py:6:2: PT006 wrong name(s) type in @pytest.mark.parametrize, expected tuple

Rule request

Description

Disallow request.addfinalizer in favor of yield-fixtures.

Bad:

@pytest.fixture()
def my_fixture(request):
    resource = acquire_resource()
    request.addfinalizer(lambda: resource.release())
    return resource

Good:

@pytest.fixture()
def my_fixture():
    resource = acquire_resource()
    yield resource
    resource.release()

Rationale

• to enforce consistency
• yield-fixtures are more simple and readable

Rule request

Description

The rule should check that there are no assert statements in fixtures.

Rationale

• assertions in fixtures are a sign of bad test design, as the test logic becomes splattered between setup/teardown code and actual test code
• inspired by eslint-plugin-jasmine rule no-expect-in-setup-teardown

If a fixture has been created with autouse=True, it likely doesn't return a value, and it's also something I should be manually specifying for any of my tests. Instead of requiring me to change this fixture names to start with an underscore, I feel like it would be better for there to be a separate rule that checks to make sure I don't manually use an autouse fixture.

Rule request

Description

I think it's a great idea to have a check which enforces the usage of specs in mocks. So either spec, spec_set or autospec should be used when using mock.Mock or mocker.Mock. Same applies to MagicMock.

Rationale

This avoids unintended existence of magic attributes which can lead to non-failing tests even if a bug is introduced.

Hey,

would you consider to add a py.typed file so that projects using this project in a programmatic way can benefit from the added type hints?
I'm happy to open a PR.

Dependabot couldn't authenticate with https://pypi.python.org/simple/.

You can provide authentication details in your Dependabot dashboard by clicking into the account menu (in the top right) and selecting 'Config variables'.

View the update logs.

Rule request

Description

I just got a PR with a change like this:

@pytest.mark.parametrize('param1', ...)
@pytest.mark.parametrize('param1', ...)
@pytest.mark.parametrize('param1', ...)
def test_...()...

Clearly the person how wrote it doesn't understand how it works. But then I thought that since multiple parameterized decorations are allowed, maybe it makes sense to lint for silly mistakes like this.

Rationale

It's for preventing typos and helping beginners to deal with rookie mistakes.

Bug report

What's wrong

In the following code:

import pytest

parameters = [
    pytest.param(1, 2, 3, 3),
    pytest.param(2, 3, 4, 4),
    pytest.param(3, 4, 5, 5),
]


class TestClass:
    @pytest.mark.parametrize(("a", "b", "_", "_2"), parameters)
    @pytest.mark.usefixtures("_")
    @pytest.mark.usefixtures("_2")
    def test_one(self, a, b):
        assert a < b

    @pytest.mark.parametrize(("_", "_2", "c", "d"), parameters)
    def test_two(self, _, _2, c, d):
        assert c == d

I get the following warning:

test/unit/test_flake.py:17:5 PT019 fixture _ without value is injected as parameter, use @pytest.mark.usefixtures instead
test/unit/test_flake.py:17:5 PT019 fixture _2 without value is injected as parameter, use @pytest.mark.usefixtures instead

This happens when we have parametric tests and we don't want to consume all parameters.
We can see that using usefixtures silences the one from test_one successfully but should it be encouraged?
This is because the parameter was mistaken for a fixture.
To be noted that PyCharm is not happy about test_one, saying Following arguments are not declared but provided by decorator: [_, _2] . It's perfectly fine with test_two.

How it should work

Parameters should be detected as parameters. And not as fixtures.
Flake8 should not request to add @pytest.mark.usefixtures (I think this is an undesirable piece of code here).
And PT019 should not be triggered.

System information

• Operating system: Fedora 35
• Python version: 3.7

pipenv run flake8 --version
3.9.2 (flake8-bugbear: 21.9.2, flake8-darglint: 1.8.1, flake8-pytest-style:
1.5.1, flake8-tidy-imports: 4.5.0, mccabe: 0.6.1, pycodestyle: 2.7.0,
pyflakes: 2.3.1) CPython 3.7.12 on Linux

Bug report

What's wrong

When using pytest-describe, PT019 (fixture without value as parameter) violations aren't reported. It's likely because these test cases don't necessarily follow the conventional naming rules (no test_ prefix) and are thus not picked up as test cases.

How it should work

Local functions inside describe_ blocks should be treated as test cases and their parameters need to be checked for violations of PT019.

System information

• Operating system: macOS
• Python version: 3.8.2
• flake8 version: 3.8.4
• flake8-pytest-style version: 1.3.0

Example

import pytest


@pytest.fixture()
def _my_fixture():
    print('Hello world')


def describe_foo():

    def it_does_stuff(_my_fixture):  # <- No PT019
        assert True


def test_foo(_my_fixture):  # <- PT019
    assert True

Output:

tests/unit/test_foo.py:15:1: PT019 fixture _my_fixture without value is injected as parameter, use @pytest.mark.usefixtures instead

Expected output:

tests/unit/test_foo.py:11:1: PT019 fixture _my_fixture without value is injected as parameter, use @pytest.mark.usefixtures instead
tests/unit/test_foo.py:15:1: PT019 fixture _my_fixture without value is injected as parameter, use @pytest.mark.usefixtures instead

Bug report

What's wrong

False positive PT004 when fixture use yield from other_function()

How it should work

I guess either flake8-pytest-style is :

• capable of determining that the function called will return a result (and includes a teardown) (recursively)
• or should only emit a warning;

But not report a false positive and exit with an error status.

System information

• Operating system:
• Python version: 3.9.5
• flake8 version: 3.9.2
• flake8-pytest-style version: 1.4.2

Rule request

Description

Test definition should not be placed in a top-level if block. pytest.mark.skipif() should be used instead, if necessary.

Bad code:

if sys.platform == 'win32':
    def test_on_windows_only():
        ...

Good code:

@pytest.mark.skipif(sys.platform != 'win32', reason='windows only')
def test_on_windows_only():
    ...

Rationale

• for consistency
• to make such conditionally skipped tests appear in the report

Bug report

What's wrong

Hi, thanks for the plugin!

I use flake8 with flake8-pytest-style as a pre-commit hook. When I check Python files containing umlauts a SyntaxError is raised.

  File "C:\Users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\Scripts\flake8.EXE\__main__.py", line 9, in <module>
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\main\cli.py", line 22, in main
    app.run(argv)
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\main\application.py", line 360, in run
    self._run(argv)
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\main\application.py", line 348, in _run
    self.run_checks()
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\main\application.py", line 262, in run_checks
    self.file_checker_manager.run()
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\checker.py", line 325, in run
    self.run_serial()
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\checker.py", line 309, in run_serial
    checker.run_checks()
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\checker.py", line 589, in run_checks
    self.run_ast_checks()
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8\checker.py", line 496, in run_ast_checks
    for (line_number, offset, text, _) in runner:
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8_plugin_utils\plugin.py", line 75, in run
    self._load_file()
  File "c:\users\tobia\.cache\pre-commit\repo__2jv1f_\py_env-default\lib\site-packages\flake8_plugin_utils\plugin.py", line 91, in _load_file
    self._tree = ast.parse(''.join(self._lines))
  File "C:\tools\miniconda3\envs\gettsim\Lib\ast.py", line 47, in parse
    return compile(source, filename, mode, flags,
  File "<unknown>", line 184
    einführungsfaktor = 0.6 + 0.02 * (
                     ^
SyntaxError: invalid character in identifier

How it should work

Parse files correctly.

System information

• Operating system: Windows
• Python version: 3.8.5
• flake8 version: 3.8.3
• flake8-pytest-style version: 1.2.2

Bug report

What's wrong

$ flake8 --pytest-parametrize-names-type tuple simple_test.py 
simple_test.py:21:2: PT006 wrong name(s) type in @pytest.mark.parametrize, expected string

If I asked for tuples, why am I bothered with strings ?

How it should work

If I ask for tuples as parameter names, there should not be any interpretation on the flake8-pytest-style side, it should only check that what I asked for is what it finds in the code:

# Assume this file is named simple_test.py
import pytest


# A string
@pytest.mark.parametrize('single_param', (
    pytest.param('hello'),
))
def test1(single_param):
    pass


# ``('single_param')`` is not a tuple, it is a parenthesized str, hence a str.
@pytest.mark.parametrize(('single_param'), (
    pytest.param('hello'),
))
def test2(single_param):
    pass


# A 1 str tuple
@pytest.mark.parametrize(('single_param', ), (
    pytest.param('hello'),
))
def test3(single_param):
    pass


# A 2 str tuple (to show that tuple length seem to be the matter)
@pytest.mark.parametrize(('single_param', 'two value'), (
    pytest.param('hello'),
))
def test4(single_param):
    pass

Then run:

$ flake8 --pytest-parametrize-names-type tuple simple_test.py 
simple_test.py:21:2: PT006 wrong name(s) type in @pytest.mark.parametrize, expected string

An issue is reported only for the one valid syntax. More over the error is confusing: I ask for tuples, I am told I should have strings.

System information

• Operating system:
• Python version:
• flake8 version:
• flake8-pytest-style version:

Dependabot can't resolve your Python dependency files.

As a result, Dependabot couldn't update your dependencies.

The error Dependabot encountered was:

Creating virtualenv flake8-pytest-style-kczUoEGx-py3.8 in /home/dependabot/.cache/pypoetry/virtualenvs
Updating dependencies
Resolving dependencies...

[PackageNotFound]
Package astpretty (1.7.0) not found.

If you think the above is an error on Dependabot's side please don't hesitate to get in touch - we'll do whatever we can to fix it.

View the update logs.

Rule request

Description

It is the best practice to remove __init__.py from tests folder.
https://docs.pytest.org/en/latest/goodpractices.html#tests-outside-application-code

Rationale

So, this linter should warn users about incorrect setup.
It is probably a good idea to make this configurable:

1. tests path
2. style: with __init__.py or without

In the official pytest documentation there are examples using pytest.fixture and not pytest.fixture()

https://docs.pytest.org/en/latest/fixture.html

# content of ./test_smtpsimple.py
import pytest


@pytest.fixture
def smtp_connection():
    import smtplib

    return smtplib.SMTP("smtp.gmail.com", 587, timeout=5)


def test_ehlo(smtp_connection):
    response, msg = smtp_connection.ehlo()
    assert response == 250
    assert 0  # for demo purposes

Input:

def my_test():
    with pytest.raises(MyError):
        with my_context_manager_under_test():
            pass

Expected: No error. This is the most minimal way to express the test without additional side effects. The test must enter the context manager to be effective.

Actual: PT012 pytest.raises() block should contain a single simple statement

We should check the same things for the new mocker.patch.context_manager that we check for other patch functions

Hi,

first of all thanks for awesome project! 👍

When updating to 1.2.0 I have spotted 1 issue, mainly, sometimes it's convenient or necessary to use "factory as a fixture" pattern.
In such cases request.addfinalizer is often the cleanest solution, because it allows to skip some kind of local registration needed when using yield statement.
To give some example, let's compare this snippet, where request.addfinalizer is used:

@pytest.fixture()
def some_factory(request):
    def factory(some_arg: str) -> SomeClass:
        some_class = SomeClass(some_arg)
        request.add_finalizer(some_class.clean_mess)
        return some_class
    return factory

with the same example, but with yield:

@pytest.fixture()
def some_factory(request):
    some_class_registry = []
    def factory(some_arg: str) -> SomeClass:
        some_class = SomeClass(some_arg)
        some_class_registry.append()
        return some_class
    yield factory
    for some_class in reversed(some_class_registry):
        some_class.clean_mess()

Currently I have used noqa in such cases and it's in majority of cases fine, because it's not so widely used pattern, but I am raising an issue, because maybe you know some better solution for this problem or it's relatively easy to implement (of course I am happy to help!)

Rule request

Description

Disallow using decorators @pytest.fixture() and @pytest.mark.asyncio at the same time.

Rationale

pytest-asyncio allows defining asynchronous fixtures, like this:

@pytest.fixture()
async def client(app: FastAPI) -> AsyncIterator[AsyncClient]:
    async with AsyncClient(app=app, base_url="http://test") as ac:
        yield ac

There is no need to put an additional mark on this fixture. But as the developer has to put @pytest.mark.asyncio mark on all the asynchronous tests, there is a chance that they'll put these redundant marks on fixtures as well:

@pytest.fixture()
@pytest.mark.asyncio  # this mark is useless on fixtures and should be disallowed
async def client(app: FastAPI) -> AsyncIterator[AsyncClient]:
    async with AsyncClient(app=app, base_url="http://test") as ac:
        yield ac

Make sure that the order of decorators doesn't matter.

Rule request

Description

Disallow unconditionally skipped tests.

Examples of bad code:

@pytest.mark.skip(reason='some reason')
def test_skipped():
    ...

@pytest.mark.skipif(True, reason='some reason')
def test_skipped():
    ...

def test_skipped():
    pytest.skip('some reason')

def test_skipped():
    if True:
        pytest.skip('some reason')

NB: this should not be reported as a violation:

def test_conditionally_skipped():
    if some_condition:
        pytest.skip('some reason')

Rationale

• to make sure skipped tests are not accidentally committed
• inspired by eslint-plugin-jest rule no-disabled-tests

Rule request

Description

This rule should check for fixtures which are decorated with pytest.mark.usefixtures. According to the docs:

Also note that this mark has no effect when applied to fixtures.

Bad example

@pytest.fixture
def a():
  pass

@pytest.mark.usefixtures("a")
@pytest.fixture
def b():
  pass

Good example

@pytest.fixture
def a():
  pass

@pytest.fixture
def b(a):
  pass

Rationale

As far as I know, pytest doesn't raise any error or waning if the above pattern is used. It can lead to weird results and broken tests, so a simple check by this plugin could help to prevent such issues.

Description

@pytest.mark.parametrize should check for iterable containers rather than enforce list.
I, for instance, prefer using immutable types, like tuples so I never use lists there. Another valid case would be using sets, which may prove to be useful if one wants to ensure there's no duplicates.

What I Did

@pytest.mark.parametrize(
	'adapter_type',
	(
		'builtin',
		'pyopenssl',
	),
)

/

@pytest.mark.parametrize(
	'is_trusted_cert,tls_client_identity',
	(
		(True, 'localhost'), (True, '127.0.0.1'),
		(True, '*.localhost'), (True, 'not_localhost'),
		(False, 'localhost'),
	),
)

Rule request

Description

Basically, whenever a deprecated fixture is encountered, the linter should warn about this and suggest a better replacement as per $sbj.

Rationale

The pytest fixtures list suggests that there's better alternatives for tmpdir and tmpdir_factory that return old-style shims:

tmp_path
Provide a pathlib.Path object to a temporary directory which is unique to each test function.
tmp_path_factory
Make session-scoped temporary directories and return pathlib.Path objects.
tmpdir
Provide a py.path.local object to a temporary directory which is unique to each test function; replaced by tmp_path.
tmpdir_factory
Make session-scoped temporary directories and return py.path.local objects; replaced by tmp_path_factory.

I saw this in a few other places in the docs but it's not emphasized enough. I often don't remember which one to use and add tmpdir expecting pathlib(2).Path objects there when I should use tmp_path instead to achieve this. Every time I have to go to docs to find out which one is old.
It's PITA and a new rule could be very helpful here, plus it could be a framework for any future deprecations that may happen later.

Bug report

When using this plugin under python3.8 or later it behaves differently than running under python3.6/python3.7.

What's wrong

Take the test project https://github.com/tox-dev/tox/tree/rewrite, using flake8-pytest-style==1.3. When running the flake8 test suite under python3.8 or later it passes with no errors. When running it under 3.7 or 3.6:

src/tox/pytest.py:66:1: PT004 fixture 'ensure_logging_framework_not_altered' does not return anything, add leading underscore
src/tox/pytest.py:111:1: PT004 fixture 'check_os_environ_stable' does not return anything, add leading underscore
src/tox/pytest.py:118:1: PT004 fixture 'no_color' does not return anything, add leading underscore
src/tox/pytest.py:274:1: PT004 fixture 'enable_pep517_backend_coverage' does not return anything, add leading underscore
src/tox/pytest.py:565:1: PT005 fixture '_invalid_index_fake_port' returns a value, remove leading underscore
tests/config/test_sets.py:13:1: PT005 fixture '_conf_builder' returns a value, remove leading underscore

How it should work

Consistently under all python versions.

System information

• Operating system: Fedora/Ubuntu
• Python version: Python3.8/3.9/3.7/3.6
• flake8 version: flake8-bugbear==20.11.1 flake8-comprehensions==3.3.1 flake8-pytest-style==1.3 flake8-spellcheck==0.23 flake8-unused-arguments==0.0.6 flake8==3.8.4

As discovered under tox-dev/tox#1970 by @jugmac00

Bug report

What's wrong

Currently I can write this code: with pytest.raises(IOError, match=''): and it passes quality check.

How it should work

This linter should warn user about empty matches.

Bad:

try:
    1 / 0
except ZeroDivisionError as e:
    assert e.message

Good:

with pytest.raises(ZeroDivisionError) as e:
    1 / 0
assert e.value.message

Rule request

Description

Catch import_module on AST walking, compare module name with conftest and trigger error on such import.

Rationale

AFAIK (but unable to prove in pytest documentation) conftest.py file intended to store fixtures/plugins/predefinded pytest methods - raw food for pytest.
Direct import of some 'test utility functions' from conftest.py could break things in unusual way.

Rule request

Description

On the wave of deprecation rules, here's another idea. The linter could check if the config is following the best practices:

1. xfail_strict = True (per https://pganssle-talks.github.io/xfail-lightning/)
2. junit_family = xunit2 (makes pytest's warnings go away)
3. addopts args on separate lines (makes it easier to annotate the purpose with comments, ref https://github.com/ansible/pylibssh/blob/a2453ed/pytest.ini#L2-L16)
4. filterwarnings should have error on the first line (rest of the lines could be legit temporary ignores)
5. -ra in addopts
6. --durations in addopts
7. minversion should be set
8. testpaths or pyargs should be specified
9. norecursedirs present

Rationale

The default pytest config is not strict enough.