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

gh-103247: add tearDown to test_loader and clear module cache #104226

Merged
merged 3 commits into from
May 10, 2023

Conversation

sunmy2019
Copy link
Member

@sunmy2019 sunmy2019 commented May 6, 2023

@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels May 6, 2023
@arhadthedev arhadthedev changed the title gh-103247: add tearDown and clear module cache gh-103247: add tearDown to test_loader and clear module cache May 6, 2023
@itamaro
Copy link
Contributor

itamaro commented May 7, 2023

I was able to repro the reported failure on main, and confirmed this PR fixes it.

I'm not sure adding the tearDown is helpful though.
the uncache context manager uncaches the names both on enter and exit, and perform some checks on the names - two things that the tearDown doesn't perform.
while having the tearDown may prevent some failures (like this one), its existence might indicate that the uncache context manager is not really needed, which is not necessarily true (I don't know if it matters in practice, just noting the behavior differences).

in any case, I also confirmed that just adding the context manager to test_try_registration without the tearDown fixes the issue.

@sunmy2019
Copy link
Member Author

I'm not sure adding the tearDown is helpful though.

Adding this is proposed by @FFY00. And I am neutral about it.

@sunmy2019
Copy link
Member Author

sunmy2019 commented May 7, 2023

I'm not sure adding the tearDown is helpful though.

I think you are correct, adding both is unnecessary.

tearDown method and util.uncache provides duplicate function. And util.uncache(name) provides more flexibility than the tearDown method.

tearDown here is a safety net. But do we want to expose the problem early or make the system more robust?

I prefer the former since it's a test case. Thus, I will remove this safe net for now.

Copy link
Contributor

@itamaro itamaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brettcannon brettcannon enabled auto-merge (squash) May 10, 2023 00:35
@brettcannon brettcannon merged commit 22f3425 into python:main May 10, 2023
@miss-islington
Copy link
Contributor

Thanks @sunmy2019 for the PR, and @brettcannon for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-104345 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label May 10, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 10, 2023
…xtensions/test_loader.py (pythonGH-104226)

(cherry picked from commit 22f3425)

Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>
@sunmy2019 sunmy2019 deleted the gh-103247 branch May 10, 2023 03:40
JelleZijlstra pushed a commit that referenced this pull request May 10, 2023
…extensions/test_loader.py (GH-104226) (#104345)

gh-103247: clear the module cache in a test in test_importlib/extensions/test_loader.py (GH-104226)
(cherry picked from commit 22f3425)

Co-authored-by: sunmy2019 <59365878+sunmy2019@users.noreply.github.com>
carljm added a commit to carljm/cpython that referenced this pull request May 10, 2023
* main:
  pythonGH-102181: Improve specialization stats for SEND (pythonGH-102182)
  pythongh-103000: Optimise `dataclasses.asdict` for the common case (python#104364)
  pythongh-103538: Remove unused TK_AQUA code (pythonGH-103539)
  pythonGH-87695: Fix OSError from `pathlib.Path.glob()` (pythonGH-104292)
  pythongh-104263: Rely on Py_NAN and introduce Py_INFINITY (pythonGH-104202)
  pythongh-104010: Separate and improve docs for `typing.get_origin` and `typing.get_args` (python#104013)
  pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic (python#104355)
  pythongh-103960: Dark mode: invert image brightness (python#103983)
  pythongh-104252: Immortalize Py_EMPTY_KEYS (pythongh-104253)
  pythongh-101819: Clean up _io windows console io after pythongh-104197 (python#104354)
  pythongh-101819: Harden _io init (python#104352)
  pythongh-103247: clear the module cache in a test in test_importlib/extensions/test_loader.py (pythonGH-104226)
  pythongh-103848: Adds checks to ensure that bracketed hosts found by urlsplit are of IPv6 or IPvFuture format (python#103849)
  pythongh-74895: adjust tests to work on Solaris (python#104326)
  pythongh-101819: Refactor _io in preparation for module isolation (python#104334)
  pythongh-90953: Don't use deprecated AST nodes in clinic.py (python#104322)
  pythongh-102327: Extend docs for "url" and "headers" parameters to HTTPConnection.request()
  pythongh-104328: Fix typo in ``typing.Generic`` multiple inheritance error message (python#104335)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants