Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PT019 and pytest-describe #74

Open
ROpdebee opened this issue Nov 2, 2020 · 3 comments
Open

PT019 and pytest-describe #74

ROpdebee opened this issue Nov 2, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@ROpdebee
Copy link
Contributor

ROpdebee commented Nov 2, 2020

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
@ROpdebee ROpdebee added the bug Something isn't working label Nov 2, 2020
@m-burst
Copy link
Owner

m-burst commented Nov 2, 2020

Hi @ROpdebee,
Thanks for the report!
I'm not really familiar with pytest-describe and its naming conventions, but I'll see what I can do to fix this.

@ROpdebee
Copy link
Contributor Author

ROpdebee commented Nov 2, 2020

As far as I know, with pytest-describe, everything that matches the following conditions is a test case:

describe-prefixes are customisable via the pytest configuration, by default the prefix is describe_.

describe blocks themselves currently cannot use fixtures (pytest-dev/pytest-describe#9 (comment)) so it's not necessary to check those for the violations, I think.

Examples:

def describe_something():  # Describe block, no need to check
    def a_test_case():  # Test case, need to check
        ...

    def _not_a_test_case():  # Not a test case, no need to check
        ...

    def describe_something_else():  # Another describe block, no need to check
        def it_works():  # Test case, need to check
            ...

I've been looking through the code a bit, utils.is_test_function() was my main area of interest but I'm not as familiar with ast as I should be, and I don't think it offers any way to get the parent of a node. I'm also not sure what the best way to handle non-standard describe-prefixes would be.

@m-burst
Copy link
Owner

m-burst commented Nov 3, 2020

I looked at pytest-describe, and it's more complicated than that.
Full support would also mean supporting tests imported into a describe block via @behaves_like.
And even if we ignore @behaves_like, there is still the issue of detecting whether pytest-describe is present and what describe-prefixes are configured.
Basic support (without @behaves_like and with hardcoded default prefixes) would be relatively easy to implement, but I'm worried that it might cause false positives in codebases that aren't even using pytest-describe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants