Skip to content

Conversation

@neutrinoceros
Copy link
Contributor

@neutrinoceros neutrinoceros commented Jan 29, 2024

Closes #11872
Thanks @bluetech for your guidance !

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]
Copy link
Contributor Author

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).

@neutrinoceros neutrinoceros marked this pull request as ready for review January 29, 2024 08:33
@RonnyPfannschmidt
Copy link
Member

wow, the inheritance tree of that exception is absolutely WILD, terrifying - i'd consider this a cpython upstream bug as well

@neutrinoceros
Copy link
Contributor Author

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.

@bluetech
Copy link
Member

@Zac-HD had fixed a similar bug in #10797, though there @Zac-HD opted to specifically check for urllib (IIUC) instead of safe getattr. So @Zac-HD WDYT?

@nicoddemus nicoddemus disabled auto-merge January 29, 2024 18:50
@Zac-HD
Copy link
Member

Zac-HD commented Jan 30, 2024

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.

Copy link
Member

@bluetech bluetech left a 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.

@neutrinoceros
Copy link
Contributor Author

Thanks for picking this up !

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: pytest.raises(HTTPError, match="...") breaks on Python 3.9 + pytest 8.0

5 participants