gh-118761: Add test_lazy_import for more modules #133057
Conversation
|
@Wulian233 I am not sure what you mean. Please see #132614 and #118761 (comment) for the motivation. |
| @@ -129,6 +129,9 @@ class StrSubclass(str): | |||
| for part in p.parts: | |||
| self.assertIs(type(part), str) | |||
|
|
|||
| def test_lazy_import(self): | |||
There was a problem hiding this comment.
Would you mind adding the @cpython_only decorator, since these tests don't necessarily make sense on other Python implementations?
I should have done that on my PRs too but didn't think about it.
|
Thanks! Feel free to make the same change for more modules in this PR. |
danielhollas
requested review from
ethanfurman,
rhettinger and
a team
as code owners
|
@JelleZijlstra thanks for taking a look! I've added the tests for Unless somebody else beats me to it, I can go through the rest of the modules handled in #118761, maybe next weekend. |
| @@ -1274,6 +1275,10 @@ class Holiday(date, Enum): | |||
| IDES_OF_MARCH = 2013, 3, 15 | |||
| self.Holiday = Holiday | |||
|
|
|||
| @cpython_only | |||
There was a problem hiding this comment.
I think this fits better in MiscTestCase (currently at around line 5286). In general I'd put this test close to test__all__ if present; it's sort of similar in testing the behavior of the whole module.
There was a problem hiding this comment.
Thanks, that's a very helpful comment as I am not familiar with cpython's test suite. Done.
danielhollas
changed the title
| @@ -129,6 +130,7 @@ class StrSubclass(str): | |||
| for part in p.parts: | |||
| self.assertIs(type(part), str) | |||
|
|
|||
| @cpython_only | |||
There was a problem hiding this comment.
This doesn't fit well in this class, can you add a new test class instead?
I went through my past PRs where I sped up the import time of
threadingandpathlibmodules (#114509 and #123520) and added regression tests to ensure that the respective module imports stay lazy, using @JelleZijlstra's newensure_lazy_importtest fixture in #132614.Happy to do this for more modules if it is better to do it in one PR instead of many.
I wasn't quite sure where to put these tests so happy to take guidance on that.