Skip to content

gh-131031: Fix separated running of pickle tests #133218

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

Closed
wants to merge 2 commits into from

Conversation

donBarbos
Copy link
Contributor

@donBarbos donBarbos commented Apr 30, 2025

I found that tests stopped running successfully directly after this commit (PR #122373)

@donBarbos donBarbos changed the title Fix separated running of pickle tests gh-131031: Fix separated running of pickle tests Apr 30, 2025
@donBarbos
Copy link
Contributor Author

cc @tomasr8 (sorry for choosing you but current pickle expert is inactive)

@tomasr8
Copy link
Member

tomasr8 commented Apr 30, 2025

Are you sure the assertions are being executed? Since self.dumps(obj, proto) raises, anything that comes after in the with block will not execute.

@donBarbos donBarbos marked this pull request as draft May 1, 2025 21:43
@donBarbos
Copy link
Contributor Author

donBarbos commented May 3, 2025

@tomasr8 sorry for the first commit.
As far as I know, changing __main__ is bad practice, but it seems that this issue cannot be solved otherwise, since the problem is that __main__ is different for different launch methods, and I was inspired by a similar PR #131371

@donBarbos donBarbos marked this pull request as ready for review May 3, 2025 17:02
@tomasr8
Copy link
Member

tomasr8 commented May 3, 2025

This looks ok, but I'm not expert in his area :/ @serhiy-storchaka could you have a look?

@jasonzitao

This comment was marked as abuse.

@jasonzitao

This comment was marked as abuse.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Good PR, but using a global class which is not imported in test_pickle.py instead of AbstractPickleTests would fix the tests too. See #133356.

Faking the __main__ module looks heavyweight, we don't know what side effects can it have. We would use this approach if there was no other way.

real_main = sys.modules.get("__main__")
fake_main = types.ModuleType("__main__")

sys.modules["__main__"] = fake_main
Copy link
Member

Choose a reason for hiding this comment

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

You can use support.swap_item(). It is already used in other tests to fake a module.

@donBarbos donBarbos closed this May 3, 2025
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.

4 participants