mock_open is not compatible with read(n) (and pickle.load)

[RonRothman]

BPO	31855
Nosy	@cjw296, @voidspace, @mariocj89, @RonRothman, @remilapeyre, @tirkarthi
PRs	bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) #11521 bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) #11521 bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) #11521 [3.7] bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) (GH-11521) #13152

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/voidspace'
closed_at = <Date 2019-05-07.13:07:44.390>
created_at = <Date 2017-10-23.21:03:10.807>
labels = ['3.7', '3.8', 'type-bug', 'library']
title = 'mock_open is not compatible with read(n) (and pickle.load)'
updated_at = <Date 2019-05-07.13:07:44.389>
user = 'https://github.com/RonRothman'

bugs.python.org fields:

activity = <Date 2019-05-07.13:07:44.389>
actor = 'xtreak'
assignee = 'michael.foord'
closed = True
closed_date = <Date 2019-05-07.13:07:44.390>
closer = 'xtreak'
components = ['Library (Lib)']
creation = <Date 2017-10-23.21:03:10.807>
creator = 'ron.rothman'
dependencies = []
files = []
hgrepos = []
issue_num = 31855
keywords = ['patch', 'patch', 'patch']
message_count = 7.0
messages = ['304841', '304846', '331920', '333484', '341704', '341714', '341719']
nosy_count = 6.0
nosy_names = ['cjw296', 'michael.foord', 'mariocj89', 'ron.rothman', 'remi.lapeyre', 'xtreak']
pr_nums = ['11521', '11521', '11521', '13152']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31855'
versions = ['Python 3.7', 'Python 3.8']

[RonRothman]

mock.mock_open works as expected when reading the entire file (read()) or when reading a single line (readline()), but it seems to not support reading a number of bytes (read(n)).

These work as expected:

    from mock import mock_open, patch

    # works: consume entire "file"
    with patch('__main__.open', mock_open(read_data='bibble')) as m:
        with open('foo') as h:
            result = h.read()

assert result == 'bibble'  # ok

    # works: consume one line
    with patch('__main__.open', mock_open(read_data='bibble\nbobble')) as m:
        with open('foo') as h:
            result = h.readline()

assert result == 'bibble\n'  # ok

But trying to read only a few bytes fails--mock_open returns the entire read_data instead:

    # consume first 3 bytes of the "file"
    with patch('__main__.open', mock_open(read_data='bibble')) as m:
        with open('foo') as h:
            result = h.read(3)

assert result == 'bib', 'result of read: {}'.format(result)  # fails

Output:

    Traceback (most recent call last):
      File "/tmp/t.py", line 25, in <module>
        assert result == 'bib', 'result of read: {}'.format(result)
    AssertionError: result of read: bibble

The unfortunate effect of this is that mock_open cannot be used with pickle.load.

    with open('/path/to/file.pkl', 'rb') as f:
        x = pickle.load(f)  # this requires f.read(1) to work

[RonRothman]

Confirmed that the behavior exists in Python 3.6 as well.

[tirkarthi]

Internally mock_open implementation uses line based iteration [0] to keep track of the state change between read calls. So readline too ignores the argument. There is bpo-25690 for an alternate mock_open implementation but the code change is large and adds a lot of features. A simpler approach would be to use StringIO or BytesIO to keep track of state changes and they provide read, readline and readlines API. mock_open docs mentions about using a customized mock for complex cases but I don't know if worthy enough to make this enhancement given the internal implementation change to support the API or to document this behavior.

Looking further there also seems to be a test case for it [1] which will fail if this is fixed since this returns all characters instead of first 10 like using open().read(10).

    def test_mock_open_read_with_argument(self):
        # At one point calling read with an argument was broken
        # for mocks returned by mock_open
        some_data = 'foo\nbar\nbaz'
        mock = mock_open(read_data=some_data)
        self.assertEqual(mock().read(10), some_data)

$ echo -n 'foo\nbar\nbaz' > /tmp/a.txt
$ ./python.exe
Python 3.8.0a0 (heads/master:f5107dfd42, Dec 16 2018, 13:41:57)
[Clang 7.0.2 (clang-700.1.81)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> with open('/tmp/a.txt') as f:
...     actual = f.read(10)
...     actual, len(actual)
...
('foo\nbar\nba', 10)
>>> with open('/tmp/a.txt') as f:
...     from unittest.mock import mock_open
...     mock = mock_open(read_data=f.read())
...     mock_data = mock().read(10)
...     mock_data, len(mock_data)
...
('foo\nbar\nbaz', 11)

[0]

cpython/Lib/unittest/mock.py

Line 2349 in f5107df

def _iterate_read_data(read_data):

[1]

cpython/Lib/unittest/test/testmock/testwith.py

Line 284 in f5107df

def test_mock_open_read_with_argument(self):

[remilapeyre]

The off-by-one error in a test added for an unrelated issue (bpo-17467) makes me think @michael.foord just made a mistake in the test.

mock_open docs mentions about using a customized mock for complex cases
I think it's more for complex things like fetching data using a callback for example, this seems like a normal use case for mock_open(). Even more so, leaving the behavior as it is now may lead someone to think there is a bug in its code.

I opened a new PR to fix this issue: #11521

[cjw296]

New changeset 11a8832 by Chris Withers (Rémi Lapeyre) in branch 'master':
bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) (GH-11521)
11a8832

[cjw296]

New changeset a6516f8 by Chris Withers (Miss Islington (bot)) in branch '3.7':
bpo-31855: unittest.mock.mock_open() results now respects the argument of read([size]) (GH-11521) (bpo-13152)
a6516f8

[tirkarthi]

PR was merged and backported to 3.7. So I am closing this as fixed. Thanks Rémi for the patch.