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

Extend pytest-raises-with-multiple-statements (PT012) to cover pytest.warns #14239

Closed
harupy opened this issue Nov 10, 2024 · 11 comments · Fixed by #15444
Closed

Extend pytest-raises-with-multiple-statements (PT012) to cover pytest.warns #14239

harupy opened this issue Nov 10, 2024 · 11 comments · Fixed by #15444
Labels
rule Implementing or modifying a lint rule

Comments

@harupy
Copy link
Contributor

harupy commented Nov 10, 2024

pytest-raises-with-multiple-statements only covers pytest.raises. It'd be nice if it covers pytest.warns as well.

import pytest


def test_raises():
    with pytest.raises(Exception):  # PT012
        f3()
        f4()


def test_warns():
    with pytest.warns(UserWarning):
        f1()  # does this warn?
        f2()  # or perhaps this one warns?
        # -> Not obvious which line warns.

Playground to ensure pytest.warns is not covered: https://play.ruff.rs/271e2f8e-751a-4368-971c-322eeeb10b39

References:

@harupy harupy changed the title Extend pytest-raises-with-multiple-statements to cover pytest.warns Extend pytest-raises-with-multiple-statements (PT012) to cover pytest.warns Nov 10, 2024
@harupy
Copy link
Contributor Author

harupy commented Nov 10, 2024

Adding a new rule is another option.

@MichaReiser
Copy link
Member

I think that would have to be its own rule unless we rename the existing rule

@harupy
Copy link
Contributor Author

harupy commented Nov 11, 2024

I think that would have to be its own rule unless we rename the existing rule

Makes sense.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label Nov 11, 2024
@harupy
Copy link
Contributor Author

harupy commented Nov 12, 2024

@MichaReiser
Copy link
Member

I think it should be fine. We just need to make sure that we use a rule code that's unlikely to collide with any new rule added to the upstream plugin

@tjkuson
Copy link
Contributor

tjkuson commented Nov 17, 2024

Created an issue in the upstream repo: m-burst/flake8-pytest-style#317

@MichaReiser
Copy link
Member

Thanks @tjkuson

@snowdrop4
Copy link
Contributor

snowdrop4 commented Nov 22, 2024

This lint makes a lot of sense to me.

I'll make a draft branch.

I'll just use a placeholder rule code until the flake8 plugin decides on one.

There could also be pytest.warns versions of PT010 and PT011 (not just PT012), since they are all analagous.

@snowdrop4
Copy link
Contributor

I have a finished branch: https://github.com/snowdrop4/ruff/tree/AVK/PytestWarnsWithMultipleStatements

But yeah, I'll wait and see what the flake8 plugin people say before PRing.

@MichaReiser
Copy link
Member

The upstream issue has been closed as completed in case someone's interested to work on this

@tjkuson
Copy link
Contributor

tjkuson commented Jan 10, 2025

I volunteer unless @snowdrop4 fancies it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants