
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2016-04-20 08:18 byrbcollins, last changed2022-04-11 14:58 byadmin. This issue is nowclosed.
| 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.pyindex 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 inLib/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.pyindex 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.251sFAILED (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 | |
./pythonLib/unittest/test/testmock/testmock.py..........................................s.....................................----------------------------------------------------------------------Ran 80 tests in 0.180sOK (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() ./pythonLib/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.197sFAILED (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 changesetcbb31b2a3dba 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/cbb31b2a3dbaNew changeset72a798e27117 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 | |