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-113407: Fix import of unittest.mock when CPython is built without docstrings #113408

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Dec 22, 2023

Copy link
Contributor

@cjw296 cjw296 left a comment

Choose a reason for hiding this comment

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

I'm worried about what needs these to be there in the first place breaking.

I'm also curious: why would you want to build a python distribution without docstrings?

if _CODE_SIG:
code_mock = NonCallableMock(spec_set=_CODE_ATTRS)
code_mock.__dict__["_spec_class"] = CodeType
code_mock.__dict__["_spec_signature"] = _CODE_SIG
Copy link
Contributor

Choose a reason for hiding this comment

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

What functionality is lost by having _spec_class and _spec_signature missing here?
These were evidently there for a reason, but if no tests are failing without them that suggest they might be superfluous and could be dropped completely, or, more likely, that there's tests coverage that needs adding.

The backport has 100% line coverage checking as part of its CI, so I'm a little surprised.

Copy link
Member Author

Choose a reason for hiding this comment

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

Was not it a pure optimization in #83076? This PR simply disables it if the signature is not available and makes the code to go the old way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I guess if there's no signature available then there's no code signature available.
Where do we go to see the test suite running (and passing after this PR!) with docstrings disabled?

@serhiy-storchaka
Copy link
Member Author

Building a python distribution without docstrings is a supported feature. Currently the mock module is broken in this configuration, and it affects many tests.

@cjw296
Copy link
Contributor

cjw296 commented Dec 24, 2023

Building a python distribution without docstrings is a supported feature

I didn't say it wasn't, I asked why would you want to build a python distribution without docstrings.
I'm still curious :-)

@serhiy-storchaka
Copy link
Member Author

I have no other reasons except supporting Python bug-free in all configurations. We do no have buildbots for all possible combinations of configuration options, so we depend on manual testing here. Time from time we test and fix builds in nonstandard configurations, but last time it was years ago. There was an old issue #81682 with an outdated PR, and it is the reason why I do this right now.

@serhiy-storchaka serhiy-storchaka merged commit 0c57454 into python:main Dec 24, 2023
28 checks passed
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the unittest-mock-without-docstrings branch December 24, 2023 11:38
@bedevere-app
Copy link

bedevere-app bot commented Dec 24, 2023

GH-113454 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 24, 2023
…thout docstrings (pythonGH-113408)

(cherry picked from commit 0c57454)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Dec 24, 2023
serhiy-storchaka added a commit that referenced this pull request Dec 24, 2023
…ithout docstrings (GH-113408) (GH-113454)

(cherry picked from commit 0c57454)

Co-authored-by: Serhiy Storchaka <[email protected]>
ryan-duve pushed a commit to ryan-duve/cpython that referenced this pull request Dec 26, 2023
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
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.

2 participants