jupyter / nbclient

A client library for executing notebooks. Formally nbconvert's ExecutePreprocessor
https://nbclient.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
152 stars 56 forks source link

Send KeyboardInterrupt a little later in test_run_all_notebooks[Interrupt.ipynb-opts6] #285

Closed kxxt closed 1 year ago

kxxt commented 1 year ago

This test is failing on riscv64 linux boards because the interrupt happens too soon. Build log: https://archriscv.felixc.at/.status/log.htm?url=logs/jupyter-nbclient/jupyter-nbclient-0.7.3-1.log

davidbrochart commented 1 year ago

I don't understand the issue. Can you run pytest -vv?

kxxt commented 1 year ago

Here is the output from pytest -vv nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6]

[root@kxxt nbclient-0.7.3]# pytest -vv nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6]
============================================================================================================ test session starts ============================================================================================================
platform linux -- Python 3.10.10, pytest-7.3.0, pluggy-1.0.0 -- /usr/bin/python
cachedir: .pytest_cache
rootdir: /build/jupyter-nbclient/src/nbclient-0.7.3
configfile: pyproject.toml
plugins: flaky-3.7.0, asyncio-0.21.0
asyncio: mode=auto
collected 1 item

nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6] FAILED                                                                                                                                                   [100%]

================================================================================================================= FAILURES ==================================================================================================================
_______________________________________________________________________________________________ test_run_all_notebooks[Interrupt.ipynb-opts6] _______________________________________________________________________________________________

input_name = 'Interrupt.ipynb', opts = {'allow_errors': True, 'interrupt_on_timeout': True, 'kernel_name': 'python', 'timeout': 1}

    @pytest.mark.parametrize(
        ["input_name", "opts"],
        [
            ("Other Comms.ipynb", {"kernel_name": "python"}),
            ("Clear Output.ipynb", {"kernel_name": "python"}),
            ("Empty Cell.ipynb", {"kernel_name": "python"}),
            ("Factorials.ipynb", {"kernel_name": "python"}),
            ("HelloWorld.ipynb", {"kernel_name": "python"}),
            ("Inline Image.ipynb", {"kernel_name": "python"}),
            (
                "Interrupt.ipynb",
                {
                    "kernel_name": "python",
                    "timeout": 1,
                    "interrupt_on_timeout": True,
                    "allow_errors": True,
                },
            ),
            ("JupyterWidgets.ipynb", {"kernel_name": "python"}),
            ("Skip Exceptions with Cell Tags.ipynb", {"kernel_name": "python"}),
            ("Skip Exceptions.ipynb", {"kernel_name": "python", "allow_errors": True}),
            ("Skip Execution with Cell Tag.ipynb", {"kernel_name": "python"}),
            ("SVG.ipynb", {"kernel_name": "python"}),
            ("Unicode.ipynb", {"kernel_name": "python"}),
            ("UnicodePy3.ipynb", {"kernel_name": "python"}),
            ("update-display-id.ipynb", {"kernel_name": "python"}),
            ("Check History in Memory.ipynb", {"kernel_name": "python"}),
        ],
    )
    def test_run_all_notebooks(input_name, opts):
        """Runs a series of test notebooks and compares them to their actual output"""
        input_file = os.path.join(current_dir, 'files', input_name)
        input_nb, output_nb = run_notebook(input_file, opts, notebook_resources())
>       assert_notebooks_equal(input_nb, output_nb)

nbclient/tests/test_client.py:346:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

expected = {'cells': [{'cell_type': 'code', 'execution_count': 1, 'metadata': {'collapsed': False}, 'outputs': [{'ename': 'Keyboa...t_type': 'stream', 'text': 'done\n'}], 'source': 'print("done")'}], 'metadata': {}, 'nbformat': 4, 'nbfor
mat_minor': 0}
actual = {'cells': [{'cell_type': 'code', 'metadata': {'collapsed': False, 'execution': {'iopub.status.busy': '2023-04-17T10:26...ents_lexer': 'ipython3', 'nbconvert_exporter': 'python', 'file_extension': '.py'}}, 'nbformat': 4, 'nbforma
t_minor': 0}

    def assert_notebooks_equal(expected, actual):
        expected_cells = expected['cells']
        actual_cells = actual['cells']
        assert len(expected_cells) == len(actual_cells)

        for expected_cell, actual_cell in zip(expected_cells, actual_cells):
            # Uncomment these to help debug test failures better
            # from pprint import pprint
            # pprint(expected_cell)
            # pprint(actual_cell)
            expected_outputs = expected_cell.get('outputs', [])
            actual_outputs = actual_cell.get('outputs', [])
            normalized_expected_outputs = list(map(normalize_output, expected_outputs))
            normalized_actual_outputs = list(map(normalize_output, actual_outputs))
>           assert normalized_expected_outputs == normalized_actual_outputs
E           AssertionError: assert [{'ename': 'KeyboardInterrupt', 'evalue': '', 'output_type': 'error', 'traceback': ['---------------------------------------------------------------------------', 'KeyboardInterrupt
    Traceback (most recent call last)', '<IPY-INPUT>\n----> 1 while True: continue\n', 'KeyboardInterrupt: ']}] == [{'output_type': 'stream', 'name': 'stderr', 'text': '\nKeyboardInterrupt\n\n'}]
E             At index 0 diff: {'ename': 'KeyboardInterrupt', 'evalue': '', 'output_type': 'error', 'traceback': ['---------------------------------------------------------------------------', 'KeyboardInterrupt                         T
raceback (most recent call last)', '<IPY-INPUT>\n----> 1 while True: continue\n', 'KeyboardInterrupt: ']} != {'output_type': 'stream', 'name': 'stderr', 'text': '\nKeyboardInterrupt\n\n'}
E             Full diff:
E               [
E             -  {'name': 'stderr',
E             +  {'ename': 'KeyboardInterrupt',
E             +   'evalue': '',
E             -   'output_type': 'stream',
E             ?                   ^^ ^^^
E             +   'output_type': 'error',
E             ?                   ^ ^^^
E             -   'text': '\n'
E             +   'traceback': ['---------------------------------------------------------------------------',
E             +                 'KeyboardInterrupt                         Traceback (most '
E             +                 'recent call last)',
E             +                 '<IPY-INPUT>\n'
E             +                 '----> 1 while True: continue\n',
E             -           'KeyboardInterrupt\n'
E             ?                             ^^
E             +                 'KeyboardInterrupt: ']},
E             ? ++++++                            ^^ +++
E             -           '\n'},
E               ]

nbclient/tests/test_client.py:288: AssertionError
------------------------------------------------------------------------------------------------------------- Captured log call -------------------------------------------------------------------------------------------------------------
ERROR    traitlets:client.py:841 Timeout waiting for execute reply (1s).
ERROR    traitlets:client.py:843 Interrupting kernel
ERROR    traitlets:client.py:841 Timeout waiting for execute reply (1s).
ERROR    traitlets:client.py:843 Interrupting kernel
============================================================================================================= warnings summary ==============================================================================================================
../../../../usr/lib/python3.10/site-packages/jupyter_client/connect.py:20
  /usr/lib/python3.10/site-packages/jupyter_client/connect.py:20: DeprecationWarning: Jupyter is migrating its paths to use standard platformdirs
  given by the platformdirs library.  To remove this warning and
  see the appropriate new directories, set the environment variable
  `JUPYTER_PLATFORM_DIRS=1` and then run `jupyter --paths`.
  The use of platformdirs will be the default in `jupyter_core` v6
    from jupyter_core.paths import jupyter_data_dir, jupyter_runtime_dir, secure_write

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================================================================================================== slowest 10 durations ============================================================================================================
10.49s call     nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6]
0.00s setup    nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6]
0.00s teardown nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6]
========================================================================================================== short test summary info ==========================================================================================================
FAILED nbclient/tests/test_client.py::test_run_all_notebooks[Interrupt.ipynb-opts6] - AssertionError: assert [{'ename': 'KeyboardInterrupt', 'evalue': '', 'output_type': 'error', 'traceback': ['-------------------------------------------
--------------------------------', 'KeyboardInterrupt                         Tra...
======================================================================================================= 1 failed, 1 warning in 19.65s =======================================================================================================
[root@kxxt nbclient-0.7.3]#

Due to the restrictive 1s timeout, the keyboard interrupt is sent before the program enters the infinite loop.

kxxt commented 1 year ago

Hi, is there any unsettled issues about this PR?

davidbrochart commented 1 year ago

Due to the restrictive 1s timeout, the keyboard interrupt is sent before the program enters the infinite loop.

I'm not convinced this is the issue. The KeyboardInterrupt is caught in both cases, but seems to be reported in a stream in one case and as an error in the other one. See https://github.com/ipython/ipykernel/issues/845.

kxxt commented 1 year ago

I'm not convinced this is the issue. The KeyboardInterrupt is caught in both cases, but seems to be reported in a stream in one case and as an error in the other one. See ipython/ipykernel#845.

Hi @davidbrochart . You are right. I added a print statement inside the while loop and it shows that the program indeed enters the infinite loop. But sending the interrupt a little later do solve the test failure, which I don't quite understand.

As you mentioned in https://github.com/ipython/ipykernel/issues/845 that you can't reproduce this issue. I can provide a way that reproduces this issue 100% of time on riscv64 emulators(Of course, it is also reproducible on real boards).

davidbrochart commented 1 year ago

I can reproduce using the setup you described, and I also don't understand. I suspect that it's related to this "machine" being slower (which could also explain why it sometimes fail in the CI), but I can't figure out where this is coming from. Let's merge for now, thanks @kxxt !