mock_open does not support iteration around text files.

[TonyFlury]

BPO	32933
Nosy	@rbtcollins, @ned-deily, @ezio-melotti, @merwok, @voidspace, @berkerpeksag, @TonyFlury, @bbayles
PRs	bpo-32933: Implement dunder iter method on mock_open #5974 3.7 Implement dunder iter method on mock_open bpo-32933 #5975 [3.7] Implement dunder iter method on mock_open bpo-32933 #5976 [3.6] : Implement dunder iter method on mock_open bpo-32933 #5977 [3.7] bpo-32933: Implement __iter__ method on mock_open() (GH-5974) #9311

^{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 = None
closed_at = <Date 2018-09-12.22:24:40.785>
created_at = <Date 2018-02-24.08:58:38.826>
labels = ['3.7', '3.8', 'type-feature', 'library']
title = 'mock_open does not support iteration around text files.'
updated_at = <Date 2018-09-14.21:30:07.650>
user = 'https://github.com/TonyFlury'

bugs.python.org fields:

activity = <Date 2018-09-14.21:30:07.650>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-12.22:24:40.785>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2018-02-24.08:58:38.826>
creator = 'anthony-flury'
dependencies = []
files = []
hgrepos = []
issue_num = 32933
keywords = ['patch']
message_count = 16.0
messages = ['312706', '315025', '315026', '320278', '321334', '321336', '325197', '325198', '325201', '325300', '325309', '325335', '325349', '325378', '325379', '325402']
nosy_count = 8.0
nosy_names = ['rbcollins', 'ned.deily', 'ezio.melotti', 'eric.araujo', 'michael.foord', 'berker.peksag', 'anthony-flury', 'bbayles']
pr_nums = ['5974', '5975', '5976', '5977', '9311']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue32933'
versions = ['Python 3.7', 'Python 3.8']

[TonyFlury]

Using the unittest.mock helper mock_open with multi-line read data, although readlines method will work on the mocked open data, the commonly used iterator idiom on an open file returns the equivalent of an empty file.

    from unittest.mock import mock_open

    read_data = 'line 1\nline 2\nline 3\nline 4\n'
    with patch('builtins.open', mock_open) as mocked:
       with open('a.txt', 'r') as fp:
           assert [l for l in StringIO(read_data)] ==
                  [l for l in fp] 

will fail although it will work on a normal file with the same data, and using [l for l in fp.readlines()] will also work.

There is a relatively simple fix which I have a working local version - but I don't know how to provide that back to the library - or even if i should.

[merwok]

Is this related to bpo-33236 ?

[TonyFlury]

No - it isn't related.

In the case of mock_open; it isn't intended to be a simple MagicMock - it is meant to be a mocked version of open, and so to be useful as a testing tool, it should emulate a file as much as possible.

When a mock_open is created, you can provide an argument 'read_data' which is meant to be the data from your mocked file, so it is key that the dunder iter method actually returns an iterator. The mock_open implementation already provides special versions of read, readline and readlines methods which use the 'read_data' initial value as the content.
Currently though the dunder iter method isn't set at all - so the returned value would currently be an empty iterator (which makes mock_open unable to be used to test idiomatic python :

    def display(file_name):
        with open('a.txt', 'r') as fp:
            for line in fp:
                print(line)  

As a trivial example the above code when mock_open is used will be equivalent of opening an empty file, but this code :

    def display(file_name):
        with open('a.txt', 'r') as fp:
            while True:
                line = readline(fp)
                if line == '':
                    break
                print(line)

Will work correctly with the data provided to mock_open.

Regardless of how and when bpo-33236 is solved - a fix would still be needed for mock_open to make it provide an iterator for the mocked file.

[ned-deily]

Anthony's PR is awaiting merge. Although Yury has reviewed it, as the core developers mocktest experts, it would be good if Michael and/or Robert could also take a look.

[berkerpeksag]

This is basically a duplicate of bpo-21258, but I haven't closely look at the patches in both issues yet.

We should probably consider adding support for __next__ as well.

[TonyFlury]

But the __next__ is a method on the iterator;

So long as __iter__ returns a valid iterator (which it does in my pull request), it will by definition support __next___

Although it is entirely possible that I have misunderstood what you are saying.

[berkerpeksag]

New changeset 2087023 by Berker Peksag (Tony Flury) in branch 'master':
bpo-32933: Implement __iter__ method on mock_open() (GH-5974)
2087023

[berkerpeksag]

Thanks for the patch, Anthony.I consider this a new feature, so I removed 3.6 and 3.7 from the versions field. We can backport to 3.7 if other core developers think that it's worth to fix in the latest maintenance branch.

[TonyFlury]

Berker,
Thanks for your work on getting this complete.

I would strongly support backporting if possible.

3.5 and 3.6 will be in common use for a while (afaik 3.6 has only now got delivered to Ubuntu as the default Python 3), and this does fix does allow full testing of what would be considered pythonic code.

[berkerpeksag]

Ned, as release manager of 3.6 and 3.7, what do you think about backporting this to maintenance releases?

[ned-deily]

While I think arguments could be made either way, this seems to me to be somewhat more of a bugfix (rather than a feature) in the sense that mock_open did not correctly emulate a real textfile open at least for an idiom that is commonly used (while acknowledging that mock_open does not claim to fully implement open or all classes of IO objects). The key question to me is would backporting this change likely cause any change in behavior to existing programs running on 3.7.x or 3.6.x. If yes, then we definitely shouldn't backport it. If not, then there is now the issue that people using mock_open on 3.7.x (or possibly 3.6.x) still can't depend on its behavior unless they explicitly check for, say, 3.7.1 or 3.6.7. That's not particularly user friendly, either. So perhaps it *is* best to not backport; if the functionality is needed in earlier releases, one could create a PyPI package to provide it, for example.

[TonyFlury]

I still support backporting to 3.6 and 3.7 :

Yes it is correct that this fix could change the behavior of existing test code, but only if someone has written a test case for a function where :

1. The function under test uses dunder_iter iteration
2. The test case provides a read_data content to mock_open
3. The test case expects a response from the function which
   implies that the file provided is empty/invalid in all cases - regardless of the data provided.

I simply cannot see that someone would implement a test case such as this - if your file has data, you would expect that your function under test would recognize that the data exists, if that data is valid; and most code will differentiate between invalid data and empty data.

So the only time I think this fix would change the behavior of existing code is if someone has written an illogical test case, which is currently passing and would now fail (since the test function will no2 see the data being provided and respond as such).

Specifically the only change in behavior to existing code is to highlight an invalid test case and potentially a bug in the code under test. It is for this reason I support backporting.

[ned-deily]

The potential change in behavior affecting existing code was one issue. But the other was the fact that people writing tests to make use of the new behavior can't depend on that behavior being there for 3.7 or 3.6 without checking the patch level, for example, 3.6.7 vs 3.6.6. That's one of the main reasons we generally do not backport behavior changes unless they are a clear bug; as I noted. this particular issue seems somewhere in between a bug and a feature. Given how far along we are in the 3.6.x cycle, I think we definitely should not backport to 3.6. Since 3.7 is near the beginning of its support cycle, I would not object if we did backport this for 3.7.1. I'll leave it up to Berker.

[berkerpeksag]

Thanks, Ned!

Anthony, I'm one of the maintainers of https://github.com/testing-cabal/mock and I'd be happy to merge a PR that backports the fix to the PyPI version of mock.

[TonyFlury]

Thank you.

[berkerpeksag]

New changeset c83c375 by Berker Peksag (Miss Islington (bot)) in branch '3.7':
bpo-32933: Implement __iter__ method on mock_open() (GH-5974)
c83c375