Issue26807
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2016-04-20 08:18 by rbcollins, last changed 2022-04-11 14:58 by admin. 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) | 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) * | Date: 2016-05-16 03:30 | |
I've committed a minimal variation - thanks for the patches. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:58:29 | admin | set | github: 70994 |
2016-05-16 03:30:31 | rbcollins | set | status: open -> closed resolution: fixed messages: + msg265662 stage: test needed -> resolved |
2016-05-16 03:23:07 | python-dev | set | nosy:
+ python-dev messages: + msg265661 |
2016-05-12 05:59:59 | rbcollins | set | messages: + msg265364 |
2016-05-12 05:13:32 | rbcollins | set | messages: + msg265362 |
2016-05-12 04:54:29 | rbcollins | set | messages: + msg265361 |
2016-04-30 08:17:14 | yolanda.robla | set | messages: + msg264545 |
2016-04-26 08:49:27 | yolanda.robla | set | messages: + msg264237 |
2016-04-25 16:03:20 | rbcollins | set | messages: + msg264183 |
2016-04-25 15:52:08 | yolanda.robla | set | messages: + msg264182 |
2016-04-25 15:42:43 | rbcollins | set | files:
+ issue26807.diff keywords: + patch messages: + msg264181 |
2016-04-25 14:52:16 | yolanda.robla | set | messages: + msg264177 |
2016-04-22 10:21:04 | rbcollins | set | messages: + msg263994 |
2016-04-20 12:40:44 | yolanda.robla | set | nosy:
+ yolanda.robla messages: + msg263826 |
2016-04-20 08:18:03 | rbcollins | create |