Skip to content

bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size])#11521

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.

Sign up for GitHub

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

Jump to bottom
Merged
cjw296 merged 4 commits into from
May 7, 2019
Merged

bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) #11521

cjw296 merged 4 commits into from
May 7, 2019

Conversation

@remilapeyre
Copy link

@remilapeyreremilapeyre commented Jan 11, 2019•
edited by bedevere-bot
Loading

@remilapeyreremilapeyre changed the title unittest.mock.mock_open() results now respects the argument of read([size])bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size])Jan 11, 2019
mariocj89
mariocj89 suggested changes May 6, 2019
View reviewed changes
Copy link
Contributor

@mariocj89mariocj89 left a comment

Choose a reason for hiding this comment

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

Great to see this fixed! 😄

Lib/unittest/mock.py Outdated

def reset_data(*args, **kwargs):
_state[0] = _iterate_read_data(read_data)
if isinstance(read_data, bytes):
Copy link
Contributor

@mariocj89mariocj89May 6, 2019

Choose a reason for hiding this comment

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

Might be good to get a function for this. Something like "_read_data_to_stream" to remove duplication.

Copy link
Author

@remilapeyreremilapeyreMay 6, 2019

Choose a reason for hiding this comment

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

Thanks :)

tirkarthi
tirkarthi reviewed May 6, 2019
View reviewed changes
@tirkarthi
Update Misc/NEWS.d/next/Library/2019-01-11-17-09-15.bpo-31855.PlhfsX.rst
b48ed55
Co-Authored-By: remilapeyre <remi.lapeyre@henki.fr>
tirkarthi
tirkarthi reviewed May 6, 2019
View reviewed changes
Lib/unittest/mock.py
forlineindata_as_list:
yieldline
def_to_stream(read_data):
ifisinstance(read_data, bytes):
Copy link
Member

@tirkarthitirkarthiMay 6, 2019

Choose a reason for hiding this comment

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

Code LGTM. I am slightly concerned if there might be any problems with backport where in Python 2 bytes is like an alias for str. So, even with BytesIO it would still be string as shown below and work as expected. Maybe I am just overthinking this.

It would be helpful if @cjw296 and @voidspace can review this since this is a good bug to be fixed. Thanks @remilapeyre .

$ python2 Python 2.7.6 (default, Sep 9 2014, 15:04:36) [GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> isinstance('a', bytes) True >>> isinstance(b'a', str) True >>> import io >>> io.BytesIO('a').readline() 'a' >>> type(io.BytesIO('a').readline()) <type 'str'>

Copy link
Contributor

@cjw296cjw296May 7, 2019

Choose a reason for hiding this comment

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

Don't worry, we can make this work in the backport :-)

Copy link
Author

@remilapeyreremilapeyreMay 7, 2019

Choose a reason for hiding this comment

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

@tirkarthi wouldn't it still be correct? The test would be useless be the mock would still work right?

Copy link
Member

@tirkarthitirkarthiMay 7, 2019

Choose a reason for hiding this comment

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

yes, in Python 2 str and bytes are the same. I will just keep on the CI once the backport lands in mock repo.

@cjw296cjw296 merged commit 11a8832 into python:masterMay 7, 2019
@miss-islington
Copy link
Contributor

miss-islington commented May 7, 2019

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 7, 2019
@miss-islington
bpo-31855: unittest.mock.mock_open() results now respects the argumen…
e461be5
…t of read([size]) (pythonGH-11521) unittest.mock.mock_open() results now respects the argument of read([size]) Co-Authored-By: remilapeyre <remi.lapeyre@henki.fr> (cherry picked from commit 11a8832) Co-authored-by: Rémi Lapeyre <remi.lapeyre@henki.fr>
@bedevere-bot
Copy link

bedevere-bot commented May 7, 2019

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

cjw296 pushed a commit that referenced this pull request May 7, 2019
@miss-islington@cjw296
bpo-31855: unittest.mock.mock_open() results now respects the argumen…
a6516f8
…t of read([size]) (GH-11521) (#13152) unittest.mock.mock_open() results now respects the argument of read([size]) Co-Authored-By: remilapeyre <remi.lapeyre@henki.fr> (cherry picked from commit 11a8832) Co-authored-by: Rémi Lapeyre <remi.lapeyre@henki.fr>
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request May 7, 2019
@CuriousLearner
Merge branch 'master' into fix-25439
2cb5fd7
* master: (1204 commits) bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) (pythonGH-11521) Forbid creating of stream objects outside of asyncio (python#13101) bpo-35925: Skip SSL tests that fail due to weak external certs. (pythonGH-13124) Fix rst formatting for several links in ssl documentation (pythonGH-13133) bpo-36542: Allow to overwrite the signature for Python functions. (pythonGH-12705) bpo-36793: Remove unneeded __str__ definitions. (pythonGH-13081) bpo-36766: Typos in docs and code comments (pythonGH-13116) bpo-36275: enhance documentation for venv.create() (pythonGH-13114) Clarify the download unit in the download section (pythonGH-13122) bpo-30668: add missing word in license.rst (pythonGH-13115) Unroll import-team in CODEOWNERS (python#13118) bpo-36594: Fix incorrect use of %p in format strings (pythonGH-12769) bpo-36798: Updating f-string docs for := use case (pythonGH-13107) Update wsgiref.rst (python#10488) Doc/c-api/exceptions.rst: fix grammar (python#12091) bpo-36811: Fix a C compiler warning in _elementtree.c. (pythonGH-13109) Only count number of members once (python#12691) bpo-16024: Doc cleanup regarding path_fd, dir_fd, follow_symlinks (pythonGH-5505) bpo-36791: Safer detection of integer overflow in sum(). (pythonGH-13080) bpo-33530: Implement Happy Eyeballs in asyncio, v2 (pythonGH-7237) ...
@cjw296
Copy link
Contributor

cjw296 commented May 7, 2019

Also in 3.0.5 of the back port: testing-cabal/mock@225d977

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjw296cjw296cjw296 left review comments

@tirkarthitirkarthitirkarthi left review comments

+1 more reviewer

@mariocj89mariocj89mariocj89 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@remilapeyre@miss-islington@bedevere-bot@cjw296@mariocj89@tirkarthi@ethanfurman@the-knights-who-say-ni