-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
BUG: fix an edge case where ExceptionInfo._stringify_exception could crash pytest.raises #11879
Conversation
0c1f196
to
60c1734
Compare
testing/acceptance_test.py
Outdated
from urllib.error import HTTPError | ||
|
||
with pytest.raises(HTTPError, match="Not Found"): | ||
raise HTTPError(code=404, msg="Not Found", fp=None, hdrs=None, url="") # type: ignore [arg-type] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I resorted to turning off mypy here because it reported that hdrs
is supposed to be of type Message
, but I have no idea where that type comes from (and it's not just an alias for str
).
60c1734
to
426fc6e
Compare
wow, the inheritance tree of that exception is absolutely WILD, terrifying - i'd consider this a cpython upstream bug as well |
Agreed, but I have limited time on my hands right now. I can't escalate this to CPython folks until Wednesday, so anyone is welcome to beat me to it. |
I think of this as implementing a workaround for a bug in old versions of CPython, and would argue against hiding bugs in exception types more generally. Better for such bugs to be noticed and fixed as quickly as possible, meaning that having pytest fail on this is actually pretty helpful! I'd suggest taking a tightly-scoped approach, which you could crib from my implementation in agronholm/exceptiongroup#58. Notably this means we'll be able to take the workaround out again when we drop support for the affected Python versions, and easily know to do so by grepping for the version comparison, without introducing new failures for other broken exception classes. |
…crash pytest.raises
Co-authored-by: Bruno Oliveira <[email protected]>
4a08322
to
1fc7425
Compare
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated to use @Zac-HD's approach, good to go now.
Thanks for picking this up ! |
Closes #11872
Thanks @bluetech for your guidance !