Bad magic unexpectedly passes

[ceball]

If I create a new notebook nb.ipynb with two cells:

In[1]:

%notmagic
x = 0
raise TypeError

In[2]:

x = 1

I would expect python -m pytest -v --nbval-lax nb.ipynb to fail, but it passes.

$ python -m pytest -v --nbval-lax nb.ipynb 
=========================================================================== test session starts ===========================================================================
platform linux -- Python 3.7.6, pytest-5.4.1, py-1.8.1, pluggy-0.12.0 -- /home/sefkw/mc3/envs/celltestsui/bin/python
cachedir: .pytest_cache
metadata: {'Python': '3.7.6', 'Platform': 'Linux-5.4.0-7634-generic-x86_64-with-debian-bullseye-sid', 'Packages': {'pytest': '5.4.1', 'py': '1.8.1', 'pluggy': '0.12.0'}, 'Plugins': {'nbval': '0.9.5', 'xdist': '1.32.0', 'html': '2.1.1', 'metadata': '1.8.0', 'cov': '2.8.1', 'forked': '1.1.2'}}
rootdir: /home/sefkw/code/external/nbval
plugins: nbval-0.9.5, xdist-1.32.0, html-2.1.1, metadata-1.8.0, cov-2.8.1, forked-1.1.2
collected 2 items                                                                                                                                                         

nb::ipynb::Cell 0 PASSED                                                                                                                                            [ 50%]
nb::ipynb::Cell 1 PASSED                                                                                                                                            [100%]

============================================================================ warnings summary =============================================================================
nbval/plugin.py:115
  /home/sefkw/code/external/nbval/nbval/plugin.py:115: PytestDeprecationWarning: direct construction of IPyNbFile has been deprecated, please use IPyNbFile.from_parent
    return IPyNbFile(path, parent)

nbval/plugin.py:312
nbval/plugin.py:312
  /home/sefkw/code/external/nbval/nbval/plugin.py:312: PytestDeprecationWarning: direct construction of IPyNbCell has been deprecated, please use IPyNbCell.from_parent
    cell, options)

nb.ipynb::Cell 0
  /home/sefkw/mc3/envs/celltestsui/lib/python3.7/site-packages/jupyter_client/manager.py:63: DeprecationWarning: KernelManager._kernel_spec_manager_changed is deprecated in traitlets 4.1: use @observe and @unobserve instead.
    def _kernel_spec_manager_changed(self):

-- Docs: https://docs.pytest.org/en/latest/warnings.html
====================================================================== 2 passed, 4 warnings in 0.55s ======================================================================

In jupyter lab, "run all" stops execution at the first cell and reports UsageError: Line magic function '%notmagic' not found

I'm replacing an existing "notebook checking tool" with nbval; that tool (based on nbconvert) also correctly reports the same failure.

[ceball]

Replacing %notmagic with e.g. %dirs, I get the expected result from nbval:

$ python -m pytest -v --nbval-lax nb.ipynb 
=============================== test session starts ================================
platform linux -- Python 3.7.6, pytest-5.4.1, py-1.8.1, pluggy-0.12.0 -- /home/sefkw/mc3/envs/celltestsui/bin/python
cachedir: .pytest_cache
metadata: {'Python': '3.7.6', 'Platform': 'Linux-5.4.0-7634-generic-x86_64-with-debian-bullseye-sid', 'Packages': {'pytest': '5.4.1', 'py': '1.8.1', 'pluggy': '0.12.0'}, 'Plugins': {'nbval': '0.9.5', 'xdist': '1.32.0', 'html': '2.1.1', 'metadata': '1.8.0', 'cov': '2.8.1', 'forked': '1.1.2'}}
rootdir: /home/sefkw/code/external/nbval
plugins: nbval-0.9.5, xdist-1.32.0, html-2.1.1, metadata-1.8.0, cov-2.8.1, forked-1.1.2
collected 2 items                                                                  

nb::ipynb::Cell 0 FAILED                                                     [ 50%]
nb::ipynb::Cell 1 PASSED                                                     [100%]

===================================== FAILURES =====================================
_________________________________ nb.ipynb::Cell 0 _________________________________
Notebook cell execution failed
Cell 0: Cell execution caused an exception

Input:
%dirs
x = 0
raise TypeError

Traceback:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-e269ed07c00d> in <module>
      1 get_ipython().run_line_magic('dirs', '')
      2 x = 0
----> 3 raise TypeError

TypeError: 

================================= warnings summary =================================
nbval/plugin.py:115
  /home/sefkw/code/external/nbval/nbval/plugin.py:115: PytestDeprecationWarning: direct construction of IPyNbFile has been deprecated, please use IPyNbFile.from_parent
    return IPyNbFile(path, parent)

nbval/plugin.py:312
nbval/plugin.py:312
  /home/sefkw/code/external/nbval/nbval/plugin.py:312: PytestDeprecationWarning: direct construction of IPyNbCell has been deprecated, please use IPyNbCell.from_parent
    cell, options)

nb.ipynb::Cell 0
  /home/sefkw/mc3/envs/celltestsui/lib/python3.7/site-packages/jupyter_client/manager.py:63: DeprecationWarning: KernelManager._kernel_spec_manager_changed is deprecated in traitlets 4.1: use @observe and @unobserve instead.
    def _kernel_spec_manager_changed(self):

-- Docs: https://docs.pytest.org/en/latest/warnings.html
============================= short test summary info ==============================
FAILED nb.ipynb::Cell 0
===================== 1 failed, 1 passed, 4 warnings in 0.59s ======================

[ceball]

For the "bad magic", you get a msg['content']['status'] of 'error' (while getting stream='shell' messages):

https://github.com/computationalmodelling/nbval/blob/3597e0bfd96c8b52d9f4e94abbe1cbb3887dcf25/nbval/kernel.py#L149-L164

(You can see that message in kernelspy in the screenshot above.)

Here, 'error' is treated no differently from any other status (except 'aborted').

Meanwhile, in runtest(), while getting outputs only from the iopub channel, the only related message is a 'stream' message.

https://github.com/computationalmodelling/nbval/blob/3597e0bfd96c8b52d9f4e94abbe1cbb3887dcf25/nbval/plugin.py#L586-L718

Here, only messages of type 'error' result in a cell error; 'stream' messages merely contribute to output.

So, no error is detected for bad magics. (Not sure if there could be other things that are missed too.)

This is in contrast to e.g. a regular python exception, which results in an iopub message of type 'error' and is hence detected here in runtest().

I'm not sure what the solution should be. E.g. I don't know if it's safe to be checking only iopub messages for errors, or if something else in the stack should be sending an iopub error message for bad magics (just like for a regular python execution failure).