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 B017 to pytest.raises #169

Closed
tsx opened this issue Apr 19, 2021 · 9 comments
Closed

Extend B017 to pytest.raises #169

tsx opened this issue Apr 19, 2021 · 9 comments

Comments

@tsx
Copy link

tsx commented Apr 19, 2021

with pytest.raises(Exception): is very similar to assertRaises, and all the justification of B017 applies to that too.

@cooperlees
Copy link
Collaborator

Sounds reasonable. Unit-tested adding to B017 will be merged.

@AbdealiLoKo
Copy link

AbdealiLoKo commented Jan 16, 2023

I have a case where I use:

with pytest.raises(Exception, match='Parsing error'):
    call_my_func()

Currently, B017 catches this as a problematic but I have a valid match here which catches a very specific error message

@cooperlees
Copy link
Collaborator

cooperlees commented Jan 16, 2023

I still feel it's a good practice to subclass Exception here too ...

If someone wants to add if match= is passed to not error, we can ...

@Dreamsorcerer
Copy link
Contributor

The error currently says Either assert for a more specific exception (builtin or custom), use assertRaisesRegex, or use the context manager form of assertRaises.

Which I assume refers to the unittest functions. But, it'd be weird to suggest using assertRaisesRegex() and then say that match is not allowed.

@mrcljx
Copy link

mrcljx commented Jan 19, 2023

I have a case where I use:

with pytest.raises(Exception, match='Parsing error'):
    call_my_func()

Currently, B017 catches this as a problematic but I have a valid match here which catches a very specific error message

@AbdealiLoKo As a workaround, you can use https://github.com/m-burst/flake8-pytest-style/blob/master/docs/rules/PT011.md to catch that.

@AbdealiLoKo
Copy link

Sure thanks @mrcljx will take a look.

@Dreamsorcerer
Copy link
Contributor

Appears this has been resolved (probably someone forgot the 'fixes' in a PR?).

@cooperlees
Copy link
Collaborator

Yup. This is all done. Thanks for the nudge.

@cooperlees
Copy link
Collaborator

FWIW - I dislike using assertRaisesRegex too, just make a custom exception type. I do accept there are edge cases where you're testing something you can't easily change etc. etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants