-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-43988: Add test.support.check_disallow_instantiation #25757
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
Conversation
cc. @vstinner |
|
||
See bpo-43916. | ||
""" | ||
msg = f"cannot create '{tp.__module__}\.{tp.__name__}' instances" |
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.
You should use f"xxx\.xxx" or fr"xxx.xxx". Or maybe use re.escape().
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.
Since these tests are decorated with cpython_only
, it should be ok to match the exact string, as produced by Objects/typeobject.c? Just matching the type module/name could in theory generate wrong result; maybe not in practice though.
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.
What about setting the match pattern depending on what test.support.check_impl_detail()
says?
Enough nitpicking, you can always enhance the code later ;-) I merged your PR, thanks. I guess that the next step is to modify existing tests to use it. |
|
It seems like more changes are coming. Once they all landed into main, maybe write a backport to 3.10 PR including all of them at once ( |
Lets leave 3.10 as it is. |
@pablogsal, should we backport this? |
I would prefer to backport it, yep |
…thonGH-25757). (cherry picked from commit 4f72526) Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
https://bugs.python.org/issue43988