Thanks Yolanda, that looks sane - could you perhaps add a test for this in Lib/unittest/tests/test_mock/ ?

Thanks Yolanda. I've touched up the test a little and run a full test run (make test) - sadly it fails: there is an explicit test that StopIteration gets raised if you set it as a side effect.

======================================================================
FAIL: test_mock_open_after_eof (unittest.test.testmock.testmock.MockTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/robertc/work/cpython-3.5.hg/Lib/unittest/test/testmock/test                                      mock.py", line 1428, in test_mock_open_after_eof
    self.assertEqual('', h.readline())
AssertionError: '' != None

======================================================================
FAIL: test_side_effect_iterator (unittest.test.testmock.testmock.MockTest )
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/robertc/work/cpython-3.5.hg/Lib/unittest/test/testmock/test                                      mock.py", line 980, in test_side_effect_iterator
    self.assertRaises(StopIteration, mock)
AssertionError: StopIteration not raised by <Mock id='140302027307384'>

======================================================================
FAIL: test_side_effect_setting_iterator (unittest.test.testmock.testmock. MockTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/robertc/work/cpython-3.5.hg/Lib/unittest/test/testmock/test                                      mock.py", line 1015, in test_side_effect_setting_iterator
    self.assertRaises(StopIteration, mock)
AssertionError: StopIteration not raised by <Mock id='140302027307720'>

Ran 705 tests in 1.251s

FAILED (failures=3, skipped=3)

Of those, I think the first failure is a bug in the patch; the second and third are genuine failures - you'll need to make your change in mock_open itself, not in 'mock'.

I've attached an updated patch which has ACKS, NEWS filled out and tweaked your test to be more comprehensive.

So... the failures are because i'm capturing the StopIteration exception in that case. then it's normal that it's not raised. How should you solve that problem instead?

You're changing _mock_call but readline is actually implemented in _readline_side_effect - just changing that should be all thats needed (to deal with StopIteration). You will however need to fixup the return values since the test I added is a bit wider than just the single defect you uncovered.

I tried patching the readline_side_effect call. But i cannot trap StopIteration there, and i don't see any clear way to detect that the generator has reached its end at that level.

How about...

@@ -2339,9 +2339,12 @@ def mock_open(mock=None, read_data=''):
         if handle.readline.return_value is not None:
             while True:
                 yield handle.readline.return_value
-        for line in _state[0]:
-            yield line
 
+        try:
+            while True:
+                yield next(_state[0])
+        except StopIteration:
+            yield ''

./python Lib/unittest/test/testmock/testmock.py
..........................................s.....................................
----------------------------------------------------------------------
Ran 80 tests in 0.180s

OK (skipped=1)

So thats great. I am going to have to stare that this quite hard to be sure I understand why it fails without the change away from 'for line in ' - but yes, this is the right place, and I'll commit it to 3.5 and 3.6 and backport it to the external mock tomorrow.

Thanks!

Actually, further inspection and a teddybear with Angus Lees uncovers this:

diff --git a/Lib/unittest/test/testmock/testmock.py b/Lib/unittest/test/testmock/testmock.py
--- a/Lib/unittest/test/testmock/testmock.py
+++ b/Lib/unittest/test/testmock/testmock.py
@@ -1419,6 +1419,18 @@
self.assertEqual('abc', first)
self.assertEqual('abc', second)

+ def test_mock_open_after_eof(self):
+ # read, readline and readlines should work after end of file.
+ _open = mock.mock_open(read_data='foo')
+ h = _open('bar')
+ h.read()
+ self.assertEqual('', h.read())
+ self.assertEqual('', h.read())
+ self.assertEqual('', h.readline())
+ self.assertEqual('', h.readline())
+ self.assertEqual([], h.readlines())
+ self.assertEqual([], h.readlines())
+
def test_mock_parents(self):
for Klass in Mock, MagicMock:
m = Klass()

./python Lib/unittest/test/testmock/testmock.py
..........................................s........E............................
======================================================================
ERROR: test_mock_open_after_eof (main.MockTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "Lib/unittest/test/testmock/testmock.py", line 1430, in test_mock_open_after_eof
    self.assertEqual('', h.readline())
  File "/home/robertc/work/cpython-3.5.hg/Lib/unittest/mock.py", line 917, in __call__
    return _mock_self._mock_call(*args, **kwargs)
  File "/home/robertc/work/cpython-3.5.hg/Lib/unittest/mock.py", line 976, in _mock_call
    result = next(effect)
StopIteration

Ran 80 tests in 0.197s

FAILED (errors=1, skipped=1)

That is, we need the yield '' to be infinite - while True: yield '', or the while True: to be outside the try:except.

So something like

  for line in _state[0]:
      yield line
  while True:
    yield ''

Will probably do it just fine.

New changeset cbb31b2a3dba by Robert Collins in branch '3.5':
Issue bpo-26807: mock_open 'files' no longer error on readline at end of file.
https://hg.python.org/cpython/rev/cbb31b2a3dba

New changeset 72a798e27117 by Robert Collins in branch 'default':
Issue bpo-26807: mock_open 'files' no longer error on readline at end of file.
https://hg.python.org/cpython/rev/72a798e27117

I've committed a minimal variation - thanks for the patches.