From 647060732bf6b60227f47c5f6e96ccf925e49ad0 Mon Sep 17 00:00:00 2001 From: Matthew Seal Date: Sat, 6 Apr 2019 21:35:37 -0700 Subject: [PATCH 1/5] Added tests for each branch in execute's run_cell method --- nbconvert/preprocessors/tests/test_execute.py | 492 +++++++++++++++++- 1 file changed, 474 insertions(+), 18 deletions(-) diff --git a/nbconvert/preprocessors/tests/test_execute.py b/nbconvert/preprocessors/tests/test_execute.py index 724673c2a..9c5b8c119 100644 --- a/nbconvert/preprocessors/tests/test_execute.py +++ b/nbconvert/preprocessors/tests/test_execute.py @@ -17,12 +17,13 @@ import nbformat import sys import pytest +import functools from .base import PreprocessorTestsBase from ..execute import ExecutePreprocessor, CellExecutionError, executenb import IPython -from mock import patch +from mock import patch, MagicMock from traitlets import TraitError from jupyter_client.kernelspec import KernelSpecManager from nbconvert.filters import strip_ansi @@ -46,7 +47,64 @@ def _normalize_base64(b64_text): except (ValueError, TypeError): return b64_text -class TestExecute(PreprocessorTestsBase): + +def merge_dicts(first, second): + # Because this is annoying to do inline + outcome = {} + outcome.update(first) + outcome.update(second) + return outcome + + +class ExecuteTestBase(PreprocessorTestsBase): + def build_preprocessor(self, opts): + """Make an instance of a preprocessor""" + preprocessor = ExecutePreprocessor() + preprocessor.enabled = True + for opt in opts: + setattr(preprocessor, opt, opts[opt]) + # Perform some state setup that should probably be in the init + preprocessor._display_id_map = {} + preprocessor.widget_state = {} + preprocessor.widget_buffers = {} + return preprocessor + + @staticmethod + def prepare_cell_mocks(*messages): + messages = list(messages) + def prepared_wrapper(func): + @functools.wraps(func) + def test_mock_wrapper(self): + parent_id = 'fake_id' + cell_mock = MagicMock(source='"foo" = "bar"', outputs=[]) + # Hack to help catch `.` and `[]` style access to outputs against the same mock object + cell_mock.__getitem__.side_effect = lambda n: cell_mock.outputs if n == 'outputs' else None + preprocessor = self.build_preprocessor({}) + preprocessor.nb = {'cells': [cell_mock]} + shell_message_mock = MagicMock( + return_value={'parent_header': {'msg_id': parent_id}}) + # Always terminate messages with an idle to exit the loop + messages.append({'msg_type': 'status', 'content': {'execution_state': 'idle'}}) + message_mock = MagicMock( + side_effect=[ + # Default the parent_header so mocks don't need to include this + merge_dicts({'parent_header': {'msg_id': parent_id}}, msg) + for msg in messages + ] + ) + channel_mock = MagicMock(get_msg=message_mock) + shell_mock = MagicMock(get_msg=shell_message_mock) + preprocessor.kc = MagicMock( + iopub_channel=channel_mock, + shell_channel=shell_mock, + execute=MagicMock(return_value=parent_id) + ) + return func(self, preprocessor, cell_mock, message_mock) + return test_mock_wrapper + return prepared_wrapper + + +class TestExecute(ExecuteTestBase): """Contains test functions for execute.py""" maxDiff = None @@ -98,15 +156,6 @@ def assert_notebooks_equal(self, expected, actual): self.assertEqual(expected_execution_count, actual_execution_count) - def build_preprocessor(self, opts): - """Make an instance of a preprocessor""" - preprocessor = ExecutePreprocessor() - preprocessor.enabled = True - for opt in opts: - setattr(preprocessor, opt, opts[opt]) - return preprocessor - - def test_constructor(self): """Can a ExecutePreprocessor be constructed?""" self.build_preprocessor({}) @@ -212,7 +261,6 @@ def test_disable_stdin(self): def test_timeout(self): """Check that an error is raised when a computation times out""" - current_dir = os.path.dirname(__file__) filename = os.path.join(current_dir, 'files', 'Interrupt.ipynb') res = self.build_resources() res['metadata']['path'] = os.path.dirname(filename) @@ -222,7 +270,6 @@ def test_timeout(self): def test_timeout_func(self): """Check that an error is raised when a computation times out""" - current_dir = os.path.dirname(__file__) filename = os.path.join(current_dir, 'files', 'Interrupt.ipynb') res = self.build_resources() res['metadata']['path'] = os.path.dirname(filename) @@ -249,7 +296,6 @@ def test_allow_errors(self): """ Check that conversion halts if ``allow_errors`` is False. """ - current_dir = os.path.dirname(__file__) filename = os.path.join(current_dir, 'files', 'Skip Exceptions.ipynb') res = self.build_resources() res['metadata']['path'] = os.path.dirname(filename) @@ -266,7 +312,6 @@ def test_force_raise_errors(self): Check that conversion halts if the ``force_raise_errors`` traitlet on ExecutePreprocessor is set to True. """ - current_dir = os.path.dirname(__file__) filename = os.path.join(current_dir, 'files', 'Skip Exceptions with Cell Tags.ipynb') res = self.build_resources() @@ -282,8 +327,6 @@ def test_force_raise_errors(self): def test_custom_kernel_manager(self): from .fake_kernelmanager import FakeCustomKernelManager - current_dir = os.path.dirname(__file__) - filename = os.path.join(current_dir, 'files', 'HelloWorld.ipynb') with io.open(filename) as f: @@ -311,7 +354,6 @@ def test_custom_kernel_manager(self): def test_execute_function(self): # Test the executenb() convenience API - current_dir = os.path.dirname(__file__) filename = os.path.join(current_dir, 'files', 'HelloWorld.ipynb') with io.open(filename) as f: @@ -350,3 +392,417 @@ def test_widgets(self): assert 'state' in d assert 'version_major' in wdata assert 'version_minor' in wdata + + +class TestRunCell(ExecuteTestBase): + """Contains test functions for ExecutePreprocessor.run_cell""" + + @ExecuteTestBase.prepare_cell_mocks() + def test_idle_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # Just the exit message should be fetched + message_mock.assert_called_once() + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'execute_reply'}, + 'parent_header': {'msg_id': 'wrong_parent'}, + 'content': {'name': 'stdout', 'text': 'foo'} + }) + def test_message_for_wrong_parent(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An ignored stream followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Ensure no output was written + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'status', + 'header': {'msg_type': 'status'}, + 'content': {'execution_state': 'busy'} + }) + def test_busy_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # One busy message, followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'execute_input', + 'header': {'msg_type': 'execute_input'}, + 'content': {} + }) + def test_execute_input_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # One ignored execute_input, followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stdout', 'text': 'foo'}, + }, { + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stderr', 'text': 'bar'} + }) + def test_stream_messages(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An stdout then stderr stream followed by an idle + self.assertEqual(message_mock.call_count, 3) + # Ensure the output was captured + self.assertListEqual(cell_mock.outputs, [ + {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'}, + {'output_type': 'stream', 'name': 'stderr', 'text': 'bar'} + ]) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'execute_reply'}, + 'content': {'name': 'stdout', 'text': 'foo'} + }, { + 'msg_type': 'clear_output', + 'header': {'msg_type': 'clear_output'}, + 'content': {} + }) + def test_clear_output_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A stream, followed by a clear, and then an idle + self.assertEqual(message_mock.call_count, 3) + # Ensure the output was cleared + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stdout', 'text': 'foo'} + }, { + 'msg_type': 'clear_output', + 'header': {'msg_type': 'clear_output'}, + 'content': {'wait': True} + }) + def test_clear_output_wait_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A stream, followed by a clear, and then an idle + self.assertEqual(message_mock.call_count, 3) + # Should be true without another message to trigger the clear + self.assertTrue(preprocessor.clear_before_next_output) + # Ensure the output wasn't cleared yet + self.assertListEqual(cell_mock.outputs, [ + {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'} + ]) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stdout', 'text': 'foo'} + }, { + 'msg_type': 'clear_output', + 'header': {'msg_type': 'clear_output'}, + 'content': {'wait': True} + }, { + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stderr', 'text': 'bar'} + }) + def test_clear_output_wait_then_message_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An stdout stream, followed by a wait clear, an stderr stream, and then an idle + self.assertEqual(message_mock.call_count, 4) + # Should be false after the stderr message + self.assertFalse(preprocessor.clear_before_next_output) + # Ensure the output wasn't cleared yet + self.assertListEqual(cell_mock.outputs, [ + {'output_type': 'stream', 'name': 'stderr', 'text': 'bar'} + ]) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'execute_reply', + 'header': {'msg_type': 'execute_reply'}, + 'content': {'execution_count': 42} + }) + def test_execution_count_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An execution count followed by an idle + self.assertEqual(message_mock.call_count, 2) + cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'execution_count': 42, 'name': 'stdout', 'text': 'foo'} + }) + def test_execution_count_with_stream_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An execution count followed by an idle + self.assertEqual(message_mock.call_count, 2) + cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + # Should also consume the message stream + self.assertListEqual(cell_mock.outputs, [ + {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'} + ]) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'comm', + 'header': {'msg_type': 'comm'}, + 'content': { + 'comm_id': 'foobar', + 'data': {'state': {'foo': 'bar'}} + } + }) + def test_widget_comm_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A comm message without buffer info followed by an idle + self.assertEqual(message_mock.call_count, 2) + self.assertEqual(preprocessor.widget_state, {'foobar': {'foo': 'bar'}}) + # Buffers should still be empty + self.assertFalse(preprocessor.widget_buffers) + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'comm', + 'header': {'msg_type': 'comm'}, + 'buffers': [b'123'], + 'content': { + 'comm_id': 'foobar', + 'data': { + 'state': {'foo': 'bar'}, + 'buffer_paths': ['path'] + } + } + }) + def test_widget_comm_buffer_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A comm message with buffer info followed by an idle + self.assertEqual(message_mock.call_count, 2) + self.assertEqual(preprocessor.widget_state, {'foobar': {'foo': 'bar'}}) + self.assertEqual(preprocessor.widget_buffers, + {'foobar': [{'data': 'MTIz', 'encoding': 'base64', 'path': 'path'}]} + ) + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'comm', + 'header': {'msg_type': 'comm'}, + 'content': { + 'comm_id': 'foobar', + # No 'state' + 'data': {'foo': 'bar'} + } + }) + def test_unknown_comm_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An unknown comm message followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Widget states should be empty as the message has the wrong shape + self.assertFalse(preprocessor.widget_state) + self.assertFalse(preprocessor.widget_buffers) + # Ensure no outputs were generated + self.assertFalse(cell_mock.outputs) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'execute_result', + 'header': {'msg_type': 'execute_result'}, + 'content': { + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'}, + 'execution_count': 42 + } + }) + def test_execute_result_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An execute followed by an idle + self.assertEqual(message_mock.call_count, 2) + cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + # Should generate an associated message + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'execute_result', + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'}, + 'execution_count': 42 + }]) + # No display id was provided + self.assertFalse(preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'execute_result', + 'header': {'msg_type': 'execute_result'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'}, + 'execution_count': 42 + } + }) + def test_execute_result_with_display_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An execute followed by an idle + self.assertEqual(message_mock.call_count, 2) + cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + # Should generate an associated message + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'execute_result', + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'}, + 'execution_count': 42 + }]) + self.assertTrue('foobar' in preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': {'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'}} + }) + def test_display_data_without_id_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A display followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Should generate an associated message + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'display_data', + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'} + }]) + # No display id was provided + self.assertFalse(preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'} + } + }) + def test_display_data_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A display followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Should generate an associated message + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'display_data', + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'} + }]) + self.assertTrue('foobar' in preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'} + } + }, { + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + } + }) + def test_display_data_same_id_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A display followed by an idle + self.assertEqual(message_mock.call_count, 3) + # Original output should be manipulated and a copy of the second now + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'display_data', + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + }, { + 'output_type': 'display_data', + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + }]) + self.assertTrue('foobar' in preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'update_display_data', + 'header': {'msg_type': 'update_display_data'}, + 'content': {'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'}} + }) + def test_update_display_data_without_id_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An update followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Display updates don't create any outputs + self.assertFalse(cell_mock.outputs) + # No display id was provided + self.assertFalse(preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'update_display_data', + 'header': {'msg_type': 'update_display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + } + }) + def test_update_display_data_mismatch_id_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An update followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Display updates don't create any outputs + self.assertFalse(cell_mock.outputs) + # Display id wasn't found, so message was skipped + self.assertFalse(preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo': 'metabar'}, + 'data': {'foo': 'bar'} + } + }, { + 'msg_type': 'update_display_data', + 'header': {'msg_type': 'update_display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + } + }) + def test_update_display_data_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # A display followed by an update then an idle + self.assertEqual(message_mock.call_count, 3) + # Original output should be manipulated + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'display_data', + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + }]) + self.assertTrue('foobar' in preprocessor._display_id_map) + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'error', + 'header': {'msg_type': 'error'}, + 'content': {'ename': 'foo', 'evalue': 'bar', 'traceback': ['Boom']} + }) + def test_error_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An error followed by an idle + self.assertEqual(message_mock.call_count, 2) + # Should also consume the message stream + self.assertListEqual(cell_mock.outputs, [{ + 'output_type': 'error', + 'ename': 'foo', + 'evalue': 'bar', + 'traceback': ['Boom'] + }]) From 1b784e8519c1fa27409e11e329926046a500430d Mon Sep 17 00:00:00 2001 From: Matthew Seal Date: Mon, 15 Apr 2019 17:51:39 -0700 Subject: [PATCH 2/5] Applied test assert changes and added additional tests based on feedback --- nbconvert/preprocessors/execute.py | 7 +- nbconvert/preprocessors/tests/test_execute.py | 208 +++++++++++------- 2 files changed, 134 insertions(+), 81 deletions(-) diff --git a/nbconvert/preprocessors/execute.py b/nbconvert/preprocessors/execute.py index 4cded0603..01c8a8120 100644 --- a/nbconvert/preprocessors/execute.py +++ b/nbconvert/preprocessors/execute.py @@ -463,7 +463,6 @@ def _wait_for_reply(self, msg_id, cell=None): self.log.error( "Kernel died while waiting for execute reply.") raise RuntimeError("Kernel died") - # kernel still alive, wait for a message continue # message received @@ -485,9 +484,9 @@ def _wait_for_reply(self, msg_id, cell=None): continue def run_cell(self, cell, cell_index=0): - msg_id = self.kc.execute(cell.source) + parent_msg_id = self.kc.execute(cell.source) self.log.debug("Executing cell:\n%s", cell.source) - exec_reply = self._wait_for_reply(msg_id, cell) + exec_reply = self._wait_for_reply(parent_msg_id, cell) outs = cell.outputs = [] self.clear_before_next_output = False @@ -506,7 +505,7 @@ def run_cell(self, cell, cell_index=0): raise RuntimeError("Timeout waiting for IOPub output") else: break - if msg['parent_header'].get('msg_id') != msg_id: + if msg['parent_header'].get('msg_id') != parent_msg_id: # not an output from our execution continue diff --git a/nbconvert/preprocessors/tests/test_execute.py b/nbconvert/preprocessors/tests/test_execute.py index 9c5b8c119..1af5730bf 100644 --- a/nbconvert/preprocessors/tests/test_execute.py +++ b/nbconvert/preprocessors/tests/test_execute.py @@ -25,6 +25,7 @@ import IPython from mock import patch, MagicMock from traitlets import TraitError +from nbformat import NotebookNode from jupyter_client.kernelspec import KernelSpecManager from nbconvert.filters import strip_ansi from testpath import modified_env @@ -71,14 +72,18 @@ def build_preprocessor(self, opts): @staticmethod def prepare_cell_mocks(*messages): + """ + TODO: describe how we're mocking overall with usage example + """ messages = list(messages) def prepared_wrapper(func): @functools.wraps(func) def test_mock_wrapper(self): + """ + TODO: describe and slit shell mock from channel mock into separate funcs + """ parent_id = 'fake_id' - cell_mock = MagicMock(source='"foo" = "bar"', outputs=[]) - # Hack to help catch `.` and `[]` style access to outputs against the same mock object - cell_mock.__getitem__.side_effect = lambda n: cell_mock.outputs if n == 'outputs' else None + cell_mock = NotebookNode(source='"foo" = "bar"', outputs=[]) preprocessor = self.build_preprocessor({}) preprocessor.nb = {'cells': [cell_mock]} shell_message_mock = MagicMock( @@ -92,7 +97,9 @@ def test_mock_wrapper(self): for msg in messages ] ) + # self.kc.iopub_channel.get_msg => message_mock.side_effect[i] channel_mock = MagicMock(get_msg=message_mock) + # self.kc.iopub_channel.get_msg => message_mock.side_effect[i] shell_mock = MagicMock(get_msg=shell_message_mock) preprocessor.kc = MagicMock( iopub_channel=channel_mock, @@ -403,7 +410,7 @@ def test_idle_message(self, preprocessor, cell_mock, message_mock): # Just the exit message should be fetched message_mock.assert_called_once() # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'stream', @@ -414,9 +421,9 @@ def test_idle_message(self, preprocessor, cell_mock, message_mock): def test_message_for_wrong_parent(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An ignored stream followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Ensure no output was written - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'status', @@ -426,9 +433,9 @@ def test_message_for_wrong_parent(self, preprocessor, cell_mock, message_mock): def test_busy_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # One busy message, followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'execute_input', @@ -438,9 +445,9 @@ def test_busy_message(self, preprocessor, cell_mock, message_mock): def test_execute_input_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # One ignored execute_input, followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'stream', @@ -454,7 +461,7 @@ def test_execute_input_message(self, preprocessor, cell_mock, message_mock): def test_stream_messages(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An stdout then stderr stream followed by an idle - self.assertEqual(message_mock.call_count, 3) + assert message_mock.call_count == 3 # Ensure the output was captured self.assertListEqual(cell_mock.outputs, [ {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'}, @@ -473,9 +480,9 @@ def test_stream_messages(self, preprocessor, cell_mock, message_mock): def test_clear_output_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A stream, followed by a clear, and then an idle - self.assertEqual(message_mock.call_count, 3) + assert message_mock.call_count == 3 # Ensure the output was cleared - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'stream', @@ -489,13 +496,13 @@ def test_clear_output_message(self, preprocessor, cell_mock, message_mock): def test_clear_output_wait_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A stream, followed by a clear, and then an idle - self.assertEqual(message_mock.call_count, 3) + assert message_mock.call_count == 3 # Should be true without another message to trigger the clear self.assertTrue(preprocessor.clear_before_next_output) # Ensure the output wasn't cleared yet - self.assertListEqual(cell_mock.outputs, [ + assert cell_mock.outputs == [ {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'} - ]) + ] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'stream', @@ -513,13 +520,37 @@ def test_clear_output_wait_message(self, preprocessor, cell_mock, message_mock): def test_clear_output_wait_then_message_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An stdout stream, followed by a wait clear, an stderr stream, and then an idle - self.assertEqual(message_mock.call_count, 4) + assert message_mock.call_count == 4 # Should be false after the stderr message - self.assertFalse(preprocessor.clear_before_next_output) + assert not preprocessor.clear_before_next_output # Ensure the output wasn't cleared yet - self.assertListEqual(cell_mock.outputs, [ + assert cell_mock.outputs == [ {'output_type': 'stream', 'name': 'stderr', 'text': 'bar'} - ]) + ] + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stdout', 'text': 'foo'} + }, { + 'msg_type': 'clear_output', + 'header': {'msg_type': 'clear_output'}, + 'content': {'wait': True} + }, { + 'msg_type': 'update_display_data', + 'header': {'msg_type': 'update_display_data'}, + 'content': {'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'}} + }) + def test_clear_output_wait_then_update_display_message(self, preprocessor, cell_mock, message_mock): + preprocessor.run_cell(cell_mock) + # An stdout stream, followed by a wait clear, an stderr stream, and then an idle + assert message_mock.call_count == 4 + # Should be false after the stderr message + assert preprocessor.clear_before_next_output + # Ensure the output wasn't cleared yet because update_display doesn't add outputs + assert cell_mock.outputs == [ + {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'} + ] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'execute_reply', @@ -529,10 +560,10 @@ def test_clear_output_wait_then_message_message(self, preprocessor, cell_mock, m def test_execution_count_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An execution count followed by an idle - self.assertEqual(message_mock.call_count, 2) - cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + assert message_mock.call_count == 2 + assert cell_mock.execution_count == 42 # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'stream', @@ -542,12 +573,12 @@ def test_execution_count_message(self, preprocessor, cell_mock, message_mock): def test_execution_count_with_stream_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An execution count followed by an idle - self.assertEqual(message_mock.call_count, 2) - cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + assert message_mock.call_count == 2 + assert cell_mock.execution_count == 42 # Should also consume the message stream - self.assertListEqual(cell_mock.outputs, [ + assert cell_mock.outputs == [ {'output_type': 'stream', 'name': 'stdout', 'text': 'foo'} - ]) + ] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'comm', @@ -560,12 +591,12 @@ def test_execution_count_with_stream_message(self, preprocessor, cell_mock, mess def test_widget_comm_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A comm message without buffer info followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 self.assertEqual(preprocessor.widget_state, {'foobar': {'foo': 'bar'}}) # Buffers should still be empty - self.assertFalse(preprocessor.widget_buffers) + assert not preprocessor.widget_buffers # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'comm', @@ -582,13 +613,13 @@ def test_widget_comm_message(self, preprocessor, cell_mock, message_mock): def test_widget_comm_buffer_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A comm message with buffer info followed by an idle - self.assertEqual(message_mock.call_count, 2) - self.assertEqual(preprocessor.widget_state, {'foobar': {'foo': 'bar'}}) - self.assertEqual(preprocessor.widget_buffers, - {'foobar': [{'data': 'MTIz', 'encoding': 'base64', 'path': 'path'}]} - ) + assert message_mock.call_count == 2 + assert preprocessor.widget_state == {'foobar': {'foo': 'bar'}} + assert preprocessor.widget_buffers == { + 'foobar': [{'data': 'MTIz', 'encoding': 'base64', 'path': 'path'}] + } # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'comm', @@ -602,12 +633,12 @@ def test_widget_comm_buffer_message(self, preprocessor, cell_mock, message_mock) def test_unknown_comm_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An unknown comm message followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Widget states should be empty as the message has the wrong shape - self.assertFalse(preprocessor.widget_state) - self.assertFalse(preprocessor.widget_buffers) + assert not preprocessor.widget_state + assert not preprocessor.widget_buffers # Ensure no outputs were generated - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'execute_result', @@ -621,17 +652,17 @@ def test_unknown_comm_message(self, preprocessor, cell_mock, message_mock): def test_execute_result_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An execute followed by an idle - self.assertEqual(message_mock.call_count, 2) - cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + assert message_mock.call_count == 2 + assert cell_mock.execution_count == 42 # Should generate an associated message - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'execute_result', 'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'}, 'execution_count': 42 - }]) + }] # No display id was provided - self.assertFalse(preprocessor._display_id_map) + assert not preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'execute_result', @@ -646,16 +677,16 @@ def test_execute_result_message(self, preprocessor, cell_mock, message_mock): def test_execute_result_with_display_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An execute followed by an idle - self.assertEqual(message_mock.call_count, 2) - cell_mock.__setitem__.assert_called_once_with('execution_count', 42) + assert message_mock.call_count == 2 + assert cell_mock.execution_count == 42 # Should generate an associated message - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'execute_result', 'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'}, 'execution_count': 42 - }]) - self.assertTrue('foobar' in preprocessor._display_id_map) + }] + assert 'foobar' in preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'display_data', @@ -665,15 +696,15 @@ def test_execute_result_with_display_message(self, preprocessor, cell_mock, mess def test_display_data_without_id_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A display followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Should generate an associated message - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'display_data', 'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'} - }]) + }] # No display id was provided - self.assertFalse(preprocessor._display_id_map) + assert not preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'display_data', @@ -687,14 +718,14 @@ def test_display_data_without_id_message(self, preprocessor, cell_mock, message_ def test_display_data_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A display followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Should generate an associated message - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'display_data', 'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'} - }]) - self.assertTrue('foobar' in preprocessor._display_id_map) + }] + assert 'foobar' in preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'display_data', @@ -704,6 +735,14 @@ def test_display_data_message(self, preprocessor, cell_mock, message_mock): 'metadata': {'metafoo': 'metabar'}, 'data': {'foo': 'bar'} } + }, { + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': { + 'transient': {'display_id': 'foobar_other'}, + 'metadata': {'metafoo_other': 'metabar_other'}, + 'data': {'foo': 'bar_other'} + } }, { 'msg_type': 'display_data', 'header': {'msg_type': 'display_data'}, @@ -716,18 +755,22 @@ def test_display_data_message(self, preprocessor, cell_mock, message_mock): def test_display_data_same_id_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A display followed by an idle - self.assertEqual(message_mock.call_count, 3) + assert message_mock.call_count == 4 # Original output should be manipulated and a copy of the second now - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'display_data', 'metadata': {'metafoo2': 'metabar2'}, 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + }, { + 'output_type': 'display_data', + 'metadata': {'metafoo_other': 'metabar_other'}, + 'data': {'foo': 'bar_other'} }, { 'output_type': 'display_data', 'metadata': {'metafoo2': 'metabar2'}, 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} - }]) - self.assertTrue('foobar' in preprocessor._display_id_map) + }] + assert 'foobar' in preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'update_display_data', @@ -737,17 +780,25 @@ def test_display_data_same_id_message(self, preprocessor, cell_mock, message_moc def test_update_display_data_without_id_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An update followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Display updates don't create any outputs - self.assertFalse(cell_mock.outputs) + assert cell_mock.outputs == [] # No display id was provided - self.assertFalse(preprocessor._display_id_map) + assert not preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'display_data', + 'header': {'msg_type': 'display_data'}, + 'content': { + 'transient': {'display_id': 'foobar'}, + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + } + }, { 'msg_type': 'update_display_data', 'header': {'msg_type': 'update_display_data'}, 'content': { - 'transient': {'display_id': 'foobar'}, + 'transient': {'display_id': 'foobar2'}, 'metadata': {'metafoo2': 'metabar2'}, 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} } @@ -755,11 +806,14 @@ def test_update_display_data_without_id_message(self, preprocessor, cell_mock, m def test_update_display_data_mismatch_id_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An update followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 3 # Display updates don't create any outputs - self.assertFalse(cell_mock.outputs) - # Display id wasn't found, so message was skipped - self.assertFalse(preprocessor._display_id_map) + assert cell_mock.outputs == [{ + 'output_type': 'display_data', + 'metadata': {'metafoo2': 'metabar2'}, + 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} + }] + assert 'foobar' in preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'display_data', @@ -781,14 +835,14 @@ def test_update_display_data_mismatch_id_message(self, preprocessor, cell_mock, def test_update_display_data_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # A display followed by an update then an idle - self.assertEqual(message_mock.call_count, 3) + assert message_mock.call_count == 3 # Original output should be manipulated - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'display_data', 'metadata': {'metafoo2': 'metabar2'}, 'data': {'foo': 'bar2', 'baz': 'foobarbaz'} - }]) - self.assertTrue('foobar' in preprocessor._display_id_map) + }] + assert 'foobar' in preprocessor._display_id_map @ExecuteTestBase.prepare_cell_mocks({ 'msg_type': 'error', @@ -798,11 +852,11 @@ def test_update_display_data_message(self, preprocessor, cell_mock, message_mock def test_error_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # An error followed by an idle - self.assertEqual(message_mock.call_count, 2) + assert message_mock.call_count == 2 # Should also consume the message stream - self.assertListEqual(cell_mock.outputs, [{ + assert cell_mock.outputs == [{ 'output_type': 'error', 'ename': 'foo', 'evalue': 'bar', 'traceback': ['Boom'] - }]) + }] From 3dcaf40ec6d3a623e0507f454a5956f88a5109bc Mon Sep 17 00:00:00 2001 From: Matthew Seal Date: Mon, 15 Apr 2019 19:07:22 -0700 Subject: [PATCH 3/5] Filled in remaining TODO items --- nbconvert/preprocessors/tests/test_execute.py | 78 ++++++++++++++----- 1 file changed, 59 insertions(+), 19 deletions(-) diff --git a/nbconvert/preprocessors/tests/test_execute.py b/nbconvert/preprocessors/tests/test_execute.py index 1af5730bf..876d19408 100644 --- a/nbconvert/preprocessors/tests/test_execute.py +++ b/nbconvert/preprocessors/tests/test_execute.py @@ -73,37 +73,77 @@ def build_preprocessor(self, opts): @staticmethod def prepare_cell_mocks(*messages): """ - TODO: describe how we're mocking overall with usage example + This function prepares a preprocessor object which has a fake kernel client + to mock the messages sent over zeromq. The mock kernel client will return + the messages passed into this wrapper back from `preproc.kc.iopub_channel.get_msg` + callbacks. It also appends a kernel idle message to the end of messages. + + This allows for testing in with following call expectations: + + @ExecuteTestBase.prepare_cell_mocks({ + 'msg_type': 'stream', + 'header': {'msg_type': 'stream'}, + 'content': {'name': 'stdout', 'text': 'foo'}, + }) + def test_message_foo(self, preprocessor, cell_mock, message_mock): + preprocessor.kc.iopub_channel.get_msg() + # => + # { + # 'msg_type': 'stream', + # 'parent_header': {'msg_id': 'fake_id'}, + # 'header': {'msg_type': 'stream'}, + # 'content': {'name': 'stdout', 'text': 'foo'}, + # } + preprocessor.kc.iopub_channel.get_msg() + # => + # { + # 'msg_type': 'status', + # 'parent_header': {'msg_id': 'fake_id'}, + # 'content': {'execution_state': 'idle'}, + # } + preprocessor.kc.iopub_channel.get_msg() # => None + message_mock.call_count # => 3 """ + parent_id = 'fake_id' messages = list(messages) + # Always terminate messages with an idle to exit the loop + messages.append({'msg_type': 'status', 'content': {'execution_state': 'idle'}}) + + def shell_channel_message_mock(): + # Return the message generator for + # self.kc.shell_channel.get_msg => {'parent_header': {'msg_id': parent_id}} + return MagicMock(return_value={'parent_header': {'msg_id': parent_id}}) + + def iopub_messages_mock(): + # Return the message generator for + # self.kc.iopub_channel.get_msg => messages[i] + return MagicMock( + side_effect=[ + # Default the parent_header so mocks don't need to include this + merge_dicts({'parent_header': {'msg_id': parent_id}}, msg) + for msg in messages + ] + ) + def prepared_wrapper(func): @functools.wraps(func) def test_mock_wrapper(self): """ - TODO: describe and slit shell mock from channel mock into separate funcs + This inner function wrapper populates the preprocessor object with + the fake kernel client. This client has it's iopub and shell + channels mocked so as to fake the setup handshake and return + the messages passed into prepare_cell_mocks as the run_cell loop + processes them. """ - parent_id = 'fake_id' cell_mock = NotebookNode(source='"foo" = "bar"', outputs=[]) preprocessor = self.build_preprocessor({}) preprocessor.nb = {'cells': [cell_mock]} - shell_message_mock = MagicMock( - return_value={'parent_header': {'msg_id': parent_id}}) - # Always terminate messages with an idle to exit the loop - messages.append({'msg_type': 'status', 'content': {'execution_state': 'idle'}}) - message_mock = MagicMock( - side_effect=[ - # Default the parent_header so mocks don't need to include this - merge_dicts({'parent_header': {'msg_id': parent_id}}, msg) - for msg in messages - ] - ) - # self.kc.iopub_channel.get_msg => message_mock.side_effect[i] - channel_mock = MagicMock(get_msg=message_mock) + # self.kc.iopub_channel.get_msg => message_mock.side_effect[i] - shell_mock = MagicMock(get_msg=shell_message_mock) + message_mock = iopub_messages_mock() preprocessor.kc = MagicMock( - iopub_channel=channel_mock, - shell_channel=shell_mock, + iopub_channel=MagicMock(get_msg=message_mock), + shell_channel=MagicMock(get_msg=shell_channel_message_mock()), execute=MagicMock(return_value=parent_id) ) return func(self, preprocessor, cell_mock, message_mock) From d06295b2a91fc9b2f57c8e3b8761eaa34a200be6 Mon Sep 17 00:00:00 2001 From: Matthew Seal Date: Mon, 15 Apr 2019 19:15:15 -0700 Subject: [PATCH 4/5] Applied last of PR feedback --- nbconvert/preprocessors/tests/test_execute.py | 16 ++++++---------- nbconvert/tests/base.py | 8 ++++++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/nbconvert/preprocessors/tests/test_execute.py b/nbconvert/preprocessors/tests/test_execute.py index 876d19408..2e4cff03f 100644 --- a/nbconvert/preprocessors/tests/test_execute.py +++ b/nbconvert/preprocessors/tests/test_execute.py @@ -23,7 +23,6 @@ from ..execute import ExecutePreprocessor, CellExecutionError, executenb import IPython -from mock import patch, MagicMock from traitlets import TraitError from nbformat import NotebookNode from jupyter_client.kernelspec import KernelSpecManager @@ -35,6 +34,10 @@ TimeoutError # Py 3 except NameError: TimeoutError = RuntimeError # Py 2 +try: + from unittest.mock import MagicMock, patch # Py 3 +except ImportError: + from mock import MagicMock, patch # Py 2 addr_pat = re.compile(r'0x[0-9a-f]{7,9}') ipython_input_pat = re.compile(r'') @@ -49,14 +52,6 @@ def _normalize_base64(b64_text): return b64_text -def merge_dicts(first, second): - # Because this is annoying to do inline - outcome = {} - outcome.update(first) - outcome.update(second) - return outcome - - class ExecuteTestBase(PreprocessorTestsBase): def build_preprocessor(self, opts): """Make an instance of a preprocessor""" @@ -120,7 +115,8 @@ def iopub_messages_mock(): return MagicMock( side_effect=[ # Default the parent_header so mocks don't need to include this - merge_dicts({'parent_header': {'msg_id': parent_id}}, msg) + ExecuteTestBase.merge_dicts( + {'parent_header': {'msg_id': parent_id}}, msg) for msg in messages ] ) diff --git a/nbconvert/tests/base.py b/nbconvert/tests/base.py index 383c94b6c..ee39dd2ca 100644 --- a/nbconvert/tests/base.py +++ b/nbconvert/tests/base.py @@ -98,6 +98,14 @@ def create_temp_cwd(self, copy_filenames=None): #Return directory handler return temp_dir + @classmethod + def merge_dicts(cls, *dict_args): + # Because this is annoying to do inline + outcome = {} + for d in dict_args: + outcome.update(d) + return outcome + def create_empty_notebook(self, path): nb = v4.new_notebook() with io.open(path, 'w', encoding='utf-8') as f: From ed25e8f6c608a2902e58c93941b0ebf96a4412ea Mon Sep 17 00:00:00 2001 From: Matthew Seal Date: Tue, 16 Apr 2019 10:16:06 -0700 Subject: [PATCH 5/5] Fixed test_execute mock for python 3.5 --- nbconvert/preprocessors/tests/test_execute.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbconvert/preprocessors/tests/test_execute.py b/nbconvert/preprocessors/tests/test_execute.py index 2e4cff03f..0ea7519a9 100644 --- a/nbconvert/preprocessors/tests/test_execute.py +++ b/nbconvert/preprocessors/tests/test_execute.py @@ -444,7 +444,7 @@ class TestRunCell(ExecuteTestBase): def test_idle_message(self, preprocessor, cell_mock, message_mock): preprocessor.run_cell(cell_mock) # Just the exit message should be fetched - message_mock.assert_called_once() + assert message_mock.call_count == 1 # Ensure no outputs were generated assert cell_mock.outputs == []