chmp / ipytest

Pytest in IPython notebooks.
MIT License
314 stars 17 forks source link

Running with ipython 8 results in crash #62

Closed ralfschulze closed 2 years ago

ralfschulze commented 2 years ago

Issue

Usint ipytest with latest ipython 8.0.1 results in program crash with ModuleNotFoundError.

Example

Simple test notebook

ipytest_nb.ipynb

import ipytest
ipytest.autoconfig()

def test_fail():
    assert False
ipytest.run()

IPython 7.23.1

(venv) rschulze@red: (dev *)~/development/pyshl$ ipython --TerminalIPythonApp.file_to_run=ipytest_nb.ipynb
F                                                                                            [100%]
============================================= FAILURES =============================================
____________________________________________ test_fail _____________________________________________

    def test_fail():
>       assert False
E       assert False

<ipython-input-1-ff73e06d7454>:2: AssertionError
===================================== short test summary info ======================================
FAILED tmp0n4g6iqq.py::test_fail - assert False
1 failed in 0.05s

IPython 8.0.1

(venv) rschulze@red: (dev *)~/development/pyshl$ ipython --TerminalIPythonApp.file_to_run=ipytest_nb.ipynb

ModuleNotFoundError                       Traceback (most recent call last)
~/development/pyshl/venv/lib/python3.8/site-packages/runipy/notebook_runner.py in <module>
     26         # IPython 3
---> 27         from IPython.kernel import KernelManager
     28         from IPython.nbformat import NotebookNode

ModuleNotFoundError: No module named 'IPython.kernel'

[...]

Solution

It seems that following replacements all-over the code solve the issue:

  1. from IPython.nbformat import... -> from nbformat import...
  2. from IPython.kernel import... -> jupyter_client import...

Probably have to be embedded in some try:... except:... for backwards compatibility.

chmp commented 2 years ago

Hi @ralfschulze,

I tried to replicate your error, but for me everything works as expected with IPython==8.0.1:

$ ipython --TerminalIPythonApp.file_to_run=tmp/Test.ipynb
F                                                                                            [100%]
============================================= FAILURES =============================================
____________________________________________ test_fail _____________________________________________

    def test_fail():
>     assert False
E     AssertionError

<ipython-input-1-b743680e3e90>:5: AssertionError
===================================== short test summary info ======================================
FAILED tmpjickvpxt.py::test_fail - AssertionError
1 failed in 0.10s

$ pip freeze | grep ipython
ipython==8.0.1
ipython-genutils==0.2.0

From the error message you posted, it seem that the issue is found somewhere in runipy. This package is not used at all in ipytest. Therefore, I would guess the error is located somewhere else. Could it be that your packages are inconsistent, i.e., that your runipy version does not work with IPython 8?

For reference: you can find the environment I tested with in the fix/62 branch.

ralfschulze commented 2 years ago

Hi @chmp,

thanks for checking so quickly!

Indeed you are right. My venv was messed up. I had installed the 'pytest-ipynb' pytest plugin for evaluation, which seemed to have cause the issue. It is not maintained anymore, so problems like this can be expected.

Sorry for that!

I noticed some other thing while testing: The nbformat dependency is not automatically installed. In a fresh venv, with only "ipython==8.0.1" and ipython-genutils==0.2.0 I get:

(venv) rschulze@red:~/0/tmp$ ipython --TerminalIPythonApp.file_to_run=ipytest_nb.ipynb
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
File ~/0/tmp/venv/lib/python3.8/site-packages/IPython/core/interactiveshell.py:2690, in InteractiveShell.safe_execfile_ipy.<locals>.get_cells()
   2688 """generator for sequence of code blocks to run"""
   2689 if fname.suffix == ".ipynb":
-> 2690     from nbformat import read
   2691     nb = read(fname, as_version=4)
   2692     if not nb.cells:

ModuleNotFoundError: No module named 'nbformat'

When I manually install nbformat everything works.

Anyhow: This issue can be closed. Thanks!

chmp commented 2 years ago

Thanks for the feedback. As for nbformat this seems to be related to how you run the notebook. ipytest does not use any functionality that requires nbformat.

For testing notebooks, I am quite happy with nbval. You can see, how it's used in ipytest here.

ralfschulze commented 2 years ago

Okay. Thanks. Running pytest --nbval-lax --current-env Example.ipynb indeed "works", but I strangely I cannot get the Example.ipynb notebook to fail. If I replace stupidly all assert's in the Example notbook with assert False I still get PASSED everywhere

What I did:

  1. virtualenv venv2 && source venv2/bin/activate
  2. python3 -m pip install ipython ipython_genutils ipytest pytest nbval (which by the way installs also nbformat as dependency)
  3. wget https://raw.githubusercontent.com/chmp/ipytest/main/Example.ipynb
  4. Replace assert.* -> assert False in Example.ipynb
  5. pytest -v --nbval-lax --current-env Example.ipynb
==================================================================== test session starts =====================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /home/rschulze/development/software/pyshl/test_nb/venv2/bin/python3
cachedir: .pytest_cache
rootdir: /home/rschulze/development/software/pyshl, configfile: pyproject.toml
plugins: nbval-0.9.6
collected 4 items                                                                                                                                            

Example.ipynb::Cell 0 PASSED                                                                                                                           [ 25%]
Example.ipynb::Cell 1 PASSED                                                                                                                           [ 50%]
Example.ipynb::Cell 2 PASSED                                                                                                                           [ 75%]
Example.ipynb::Cell 3 PASSED                                                                                                                           [100%]

===================================================================== 4 passed in 1.19s ======================================================================

When I add assert ipytest.exit_code == 0 at the end I get at least

[...]
Example.ipynb::Cell 0 PASSED                                                                                                                           [ 25%]
Example.ipynb::Cell 1 PASSED                                                                                                                           [ 50%]
Example.ipynb::Cell 2 PASSED                                                                                                                           [ 75%]
Example.ipynb::Cell 3 FAILED                                                                                                                           [100%]
[...]

so only the exit_code assert fails, while the rest is reported as PASSED.

I am sorry if I do anything terribly wrong, but I am currently scratching my head what the problem might be.

chmp commented 2 years ago

There is nothing you're doing wrong. The issue is that ipytest does not raise an exception if your tests fail. This behavior is by design, as in an interactive sessions the raised exception would come on top of the usual pytest output and having multiple competing stack traces could be quite confusing.

If you would like to raise an exception you have to manually check for the return code of pytest, as you did above. At some point there was a built in mechanism for this setup, but my impression was that nobody was using it.

The reason why only the last cell fails is that you would need to check all cells independently, since each %%ipytest magic overwrites the previous value of the exit_code. Importantly, the ipytest.exit_code is only set after the cell has been executed, so you would need to add a second cell where you test it. Or use assert ipytest.run() == 0. It depends a bit on what you are looking to acomplish, but it seems your usecase would be quite well served by writing the notebook however you like and include a call to ipytest.run as the very last cell.

ralfschulze commented 2 years ago

Are you sure about the ipytest.run() == 0 line? In my tests the run() method always give None and there does not seem to be a return statement in run() https://github.com/chmp/ipytest/blob/b9d8dff02fddafe0f9a1742b76cbac20d406994a/ipytest/_impl.py#L22

What I want to achieve is to run automated unit-tests in my notebook when I push it to our CI/CD.

I tried to run the notebook via ipython --InteractiveShellApp.code_to_run='%run TestNotebook.ipynb. That works in the sense that all assertions are reported, but ipython does not set an exit code != 0 when assertions fail, so this does not work for CI/CD.

Your solution via pytest --nbval-lax --current-env TestNotebook.ipynb gives the clear pytest output, but I have to put assert ipytest.exit_code == 0 at the end of each cell, or the tests will always pass. I would prefer to have this checked automatically, but the line is easy enough to add (Just have to remember it :-), so I will try to use this solution for my problem.

Anyhow: Thanks for sharing this package and also for your support!

chmp commented 2 years ago

@ralfschulze you're right, the return code is not returned. I released ipytest==0.12.0b1 which offers two options to make sure test failures lead to exceptions (see https://github.com/chmp/ipytest/issues/63):

  1. Use ipytest.autoconfig(raise_on_error=True)
  2. Use assert ipytest.run() == 0

It would be cool, if you could proide feedback, whether the changes work for your use case.

ralfschulze commented 2 years ago

Yes! raise_on_error works fine for me and does exactly what I was looking for.

For assert ipytest.run() == 0 it seems it has to be called at the end of each cell to do what I want. Doing it once at the end of the notebook is not sufficient, so I will use the raise_on_error option, which I find easier to use.

Very nice. Thanks a lot!

chmp commented 2 years ago

Perfect. Thanks for the feedback. As mentioned in https://github.com/chmp/ipytest/issues/63, I will create a proper release after testing it a bit myself.

chmp commented 2 years ago

Released as https://pypi.org/project/ipytest/0.12.0/