[issue32933] mock_open does not support iteration around text files.

[Berker Peksag]

Berker Peksag  added the comment:


New changeset c83c375ed907bdd54361aa36ce76130360f323a4 by Berker Peksag (Miss 
Islington (bot)) in branch '3.7':
bpo-32933: Implement __iter__ method on mock_open() (GH-5974)
https://github.com/python/cpython/commit/c83c375ed907bdd54361aa36ce76130360f323a4


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Anthony Flury  added the comment:

Thank you.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Berker Peksag]

Berker Peksag  added the comment:

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.

--
versions: +Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[miss-islington]

Change by miss-islington :


--
pull_requests: +8737

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Ned Deily]

Ned Deily  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Anthony Flury  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Ned Deily]

Ned Deily  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Berker Peksag]

Berker Peksag  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Anthony Flury  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Berker Peksag]

Berker Peksag  added the comment:

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.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
type: behavior -> enhancement
versions:  -Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Berker Peksag]

Berker Peksag  added the comment:


New changeset 2087023fdec2c89070bd14f384a3c308c548a94a by Berker Peksag (Tony 
Flury) in branch 'master':
bpo-32933: Implement __iter__ method on mock_open() (GH-5974)
https://github.com/python/cpython/commit/2087023fdec2c89070bd14f384a3c308c548a94a


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Anthony Flury  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Berker Peksag]

Berker Peksag  added the comment:

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.

--
nosy: +berker.peksag

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Ned Deily]

Ned Deily  added the comment:

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.

--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Anthony Flury  added the comment:

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 #33236 is solved - a fix would still be needed for 
mock_open to make it provide an iterator for the mocked file.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Éric Araujo]

Éric Araujo  added the comment:

Is this related to #33236 ?

--
nosy: +eric.araujo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Change by Anthony Flury :


--
pull_requests: +5743

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[bbayles]

Change by bbayles :


--
nosy: +bbayles

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Change by Anthony Flury :


--
pull_requests: +5742

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Change by Anthony Flury :


--
pull_requests: +5741

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Change by Anthony Flury :


--
keywords: +patch
pull_requests: +5740
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Terry J. Reedy]

Change by Terry J. Reedy :


--
nosy: +ezio.melotti, michael.foord, rbcollins
versions: +Python 3.8 -Python 3.4, Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

Change by Anthony Flury :


--
type:  -> behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32933] mock_open does not support iteration around text files.

[Anthony Flury]

New submission from Anthony Flury :

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.

--
components: Library (Lib)
messages: 312706
nosy: anthony-flury
priority: normal
severity: normal
status: open
title: mock_open does not support iteration around text files.
versions: Python 3.4, Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com