classification
Title: mock_open()().readline() fails at EOF
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: python-dev, rbcollins, yolanda.robla
Priority: normal Keywords: patch

Created on 2016-04-20 08:18 by rbcollins, last changed 2016-05-16 03:30 by rbcollins. This issue is now closed.

Files
File name Uploaded Description Edit
issue26807.diff rbcollins, 2016-04-25 15:42
Messages (14)
msg263814 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-04-20 08:18
>>> import unittest.mock
>>> o = unittest.mock.mock_open(read_data="fred")
>>> f = o()
>>> f.read()
'fred'
>>> f.read()
''
>>> f.readlines()
[]
>>> f.readline()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/robertc/work/cpython/Lib/unittest/mock.py", line 935, in __call__
    return _mock_self._mock_call(*args, **kwargs)
  File "/home/robertc/work/cpython/Lib/unittest/mock.py", line 994, in _mock_call
    result = next(effect)
StopIteration
msg263826 - (view) Author: Yolanda (yolanda.robla) Date: 2016-04-20 12:40
diff --git a/Lib/unittest/mock.py b/Lib/unittest/mock.py
index 7400fb7..9e47cd2 100644
--- a/Lib/unittest/mock.py
+++ b/Lib/unittest/mock.py
@@ -991,7 +991,10 @@ class CallableMixin(Base):
                 raise effect
 
             if not _callable(effect):
-                result = next(effect)
+                try:
+                    result = next(effect)
+                except StopIteration:
+                    result = None
                 if _is_exception(result):
                     raise result
                 if result is DEFAULT:
msg263994 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-04-22 10:21
Thanks Yolanda, that looks sane - could you perhaps add a test for this in Lib/unittest/tests/test_mock/ ?
msg264177 - (view) Author: Yolanda (yolanda.robla) Date: 2016-04-25 14:52
diff --git a/Lib/unittest/test/testmock/testwith.py b/Lib/unittest/test/testmock/testwith.py
index a7bee73..efe6391 100644
--- a/Lib/unittest/test/testmock/testwith.py
+++ b/Lib/unittest/test/testmock/testwith.py
@@ -297,5 +297,16 @@ class TestMockOpen(unittest.TestCase):
         self.assertEqual(handle.readline(), 'bar')
 
 
+    def test_readlines_after_eof(self):
+        # Check that readlines does not fail after the end of file
+        mock = mock_open(read_data='foo')
+        with patch('%s.open' % __name__, mock, create=True):
+            h = open('bar')
+            h.read()
+            h.read()
+            data = h.readlines()
+            self.assertEqual(data, [])
+
+
 if __name__ == '__main__':
     unittest.main()
msg264181 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-04-25 15:42
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.
msg264182 - (view) Author: Yolanda (yolanda.robla) Date: 2016-04-25 15:52
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?
msg264183 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-04-25 16:03
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.
msg264237 - (view) Author: Yolanda (yolanda.robla) Date: 2016-04-26 08:49
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.
msg264545 - (view) Author: Yolanda (yolanda.robla) Date: 2016-04-30 08:17
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 ''
msg265361 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-05-12 04:54
./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!
msg265362 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-05-12 05:13
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.
msg265364 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-05-12 05:59
So something like

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

Will probably do it just fine.
msg265661 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-05-16 03:23
New changeset cbb31b2a3dba by Robert Collins in branch '3.5':
Issue #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 #26807: mock_open 'files' no longer error on readline at end of file.
https://hg.python.org/cpython/rev/72a798e27117
msg265662 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-05-16 03:30
I've committed a minimal variation - thanks for the patches.
History
Date User Action Args
2016-05-16 03:30:31rbcollinssetstatus: open -> closed
resolution: fixed
messages: + msg265662

stage: test needed -> resolved
2016-05-16 03:23:07python-devsetnosy: + python-dev
messages: + msg265661
2016-05-12 05:59:59rbcollinssetmessages: + msg265364
2016-05-12 05:13:32rbcollinssetmessages: + msg265362
2016-05-12 04:54:29rbcollinssetmessages: + msg265361
2016-04-30 08:17:14yolanda.roblasetmessages: + msg264545
2016-04-26 08:49:27yolanda.roblasetmessages: + msg264237
2016-04-25 16:03:20rbcollinssetmessages: + msg264183
2016-04-25 15:52:08yolanda.roblasetmessages: + msg264182
2016-04-25 15:42:43rbcollinssetfiles: + issue26807.diff
keywords: + patch
messages: + msg264181
2016-04-25 14:52:16yolanda.roblasetmessages: + msg264177
2016-04-22 10:21:04rbcollinssetmessages: + msg263994
2016-04-20 12:40:44yolanda.roblasetnosy: + yolanda.robla
messages: + msg263826
2016-04-20 08:18:03rbcollinscreate