[proxy] github.com — ← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Member

tirkarthi commented Jan 18, 2019 •
edited by bedevere-bot
Loading

Add support for staticmethod and classmethod to mock created with autospec. Patch by @felipeochoa converted into a PR.

Co-authored-by: Felipe felipe.nospam.ochoa@gmail.com

https://bugs.python.org/issue23078

tirkarthi and others added 2 commits January 18, 2019 23:06
Add support for staticmethod and classmethod to mock.
3682bdb 
Co-authored-by: Felipe <felipe.nospam.ochoa@gmail.com>
Add NEWS entry.
4438f0e 
Co-authored-by: Felipe <felipe.nospam.ochoa@gmail.com>
Copy link
Member Author

tirkarthi commented Feb 24, 2019

@mariocj89 @cjw296 Friendly ping. I have converted the patch from the tracker to PR attributing to original author. It was a frequently requested mock feature and I would be happy to act on review comments for this one.

Thanks.

mariocj89 approved these changes Feb 25, 2019
View reviewed changes
Copy link
Contributor

mariocj89 left a comment

Choose a reason for hiding this comment

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

Great!, small nitpicks if you want to pick them.

Lib/unittest/test/testmock/testmock.py Outdated
#Issue23078
def test_create_autospec_staticmethod(self):
class TestClass:
method = staticmethod(lambda: None)
Copy link
Contributor

mariocj89 Feb 25, 2019

Choose a reason for hiding this comment

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

It might be easier to read with the common syntax decorating the method.

Copy link
Member Author

tirkarthi Feb 26, 2019

Choose a reason for hiding this comment

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

Thanks, changed to decorators 👍

Misc/NEWS.d/next/Library/2019-01-18-23-10-10.bpo-23078.l4dFoj.rst Outdated
@@ -0,0 +1,2 @@
Add support for staticmethod and classmethod with autospec. Based on patch
by Felipe.
Copy link
Contributor

mariocj89 Feb 25, 2019

Choose a reason for hiding this comment

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

You might want to include Felipe's surname. And you can also get credit for this :).

Copy link
Member Author

tirkarthi Feb 26, 2019

Choose a reason for hiding this comment

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

Thanks, made the changes. cc : @felipeochoa

mariocj89 approved these changes Feb 26, 2019
View reviewed changes
Copy link
Contributor

mariocj89 left a comment

Choose a reason for hiding this comment

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

Great! 😄

berkerpeksag merged commit 9b21856 into python:master Apr 22, 2019
Copy link
Contributor

miss-islington commented Apr 22, 2019

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

Copy link
Contributor

miss-islington commented Apr 22, 2019

Sorry, @tirkarthi and @berkerpeksag, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 9b21856b0fcda949de239edc7aa6cf3f2f4f77a3 3.7

berkerpeksag pushed a commit to berkerpeksag/cpython that referenced this pull request Apr 22, 2019
bpo-23078: Add support for {class,static}method to mock.create_autosp…
4653fda 
…ec() (pythonGH-11613)

Co-authored-by: Felipe <felipe.nospam.ochoa@gmail.com>

(cherry picked from commit 9b21856)
Copy link

bedevere-bot commented Apr 22, 2019

GH-12901 is a backport of this pull request to the 3.7 branch.

berkerpeksag added a commit that referenced this pull request Apr 22, 2019
bpo-23078: Add support for {class,static}method to mock.create_autosp…
15a57a3 
…ec() (GH-11613)

Co-authored-by: Felipe <felipe.nospam.ochoa@gmail.com>

(cherry picked from commit 9b21856)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

berkerpeksag berkerpeksag approved these changes

+1 more reviewer

mariocj89 mariocj89 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

berkerpeksag

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants