Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skipping Notebook at runtime using pytest.exit("Not applicable on Frobble [skip-notebook]") #43

Open
amotl opened this issue Oct 29, 2023 · 5 comments

Comments

@amotl
Copy link
Contributor

amotl commented Oct 29, 2023

Dear Chris,

thanks a stack for conceiving this excellent package.

We are using pytest-notebook with the JupyterNbCollector, and investigated the capabilities how to conditionally skip notebooks while testing, but only found it to be possible using Notebook/Cell Metadata.

In order to skip notebooks at runtime, based on other conditions, we will now detour to using a pytest-based test file, and invoke the notebooks using NBRegressionFixture. We guess this is the right choice?

However, it looks like it may also be made possible to actually skip notebooks at runtime. This notebook code outlines that pytest's Skipped exception is already raised and propagated properly, so corresponding code in pytest-notebook would only need to handle it appropriately?

import pytest
if "ACME_API_KEY" not in os.environ:
    pytest.skip("No ACME API key given", allow_module_level=True)
Cell In[3], line 11
---> 11 pytest.skip("No ACME API key given", allow_module_level=True)

File ~/path/to/.venv/lib/python3.11/site-packages/_pytest/outcomes.py:179, in skip(reason, allow_module_level, msg)
    177 __tracebackhide__ = True
    178 reason = _resolve_msg_to_reason("skip", reason, msg)
--> 179 raise Skipped(msg=reason, allow_module_level=allow_module_level)

Skipped: No ACME API key given

With kind regards,
Andreas.

@amotl
Copy link
Contributor Author

amotl commented Oct 29, 2023

For CoverageNotebookClient.async_execute, we have been able to come up with this patch. It will re-raise pytest's Skipped exception, and is apparently doing what we want.

However, we don't know whether it is actually legit, and if you would accept a corresponding patch?

for index, cell in enumerate(self.nb.cells):
    try:
        await self.async_execute_cell(
            cell, index, execution_count=self.code_cells_executed + 1
        )
    except CellExecutionError as ex:
        if ex.ename == "Skipped":
            raise pytest.skip(ex.evalue)
        else:
            raise

@amotl
Copy link
Contributor Author

amotl commented Oct 29, 2023

Hi again,

we are now using a little of monkeypatching code in our conftest.py.

Instead of using pytest.skip(), as outlined above, it will propagate cell-level pytest.exit() invocations as signals to mark the whole notebook as skipped.

In order not to be too intrusive, the feature only skips notebooks when being explicitly instructed, by adding [skip-notebook] at the end of the reason string. Example:

import os
import pytest

if "ACME_API_KEY" not in os.environ:
    pytest.exit("ACME_API_KEY not given [skip-notebook]")

We actually don't expect this "addon" to be mainlined in any way. Do you have any other suggestions how to proceed with this?

With kind regards,
Andreas.

@amotl amotl changed the title Skipping Notebook at runtime Skipping Notebook at runtime using pytest.exit("Not applicable on Frobble [skip-notebook]") Oct 29, 2023
@chrisjsewell
Copy link
Owner

Hey hey! Cheers I'll try and have a look soon 😄

@amotl
Copy link
Contributor Author

amotl commented Oct 29, 2023

Another variant for the API, not relying on pytest.exit(), would be to make it more explicit, only building upon corresponding primitives from pytest_notebook itself.

Let's consider this more elaborate snippet from our workbench where it is also made clear that "notebook skipping" is only a thing when actually running under pytest.

import os
import getpass

# Skip testing notebook if API key is not supplied.
# When running interactively, prompt.
if "ACME_API_KEY" not in os.environ:
    if "PYTEST_CURRENT_TEST" in os.environ:
        import pytest_notebook
        pytest_notebook.skip_notebook("ACME_API_KEY not given")
    else:
        os.environ["ACME_API_KEY"] = getpass.getpass("ACME API key:")

The exception propagation and handling would probably be the same, only using a custom exception type this time, unless you are able to come up with a totally different mechanism.

class SkipNotebook(Exception):
    """
    Custom exception type to send a signal from the notebook to pytest-notebook
    to stop test execution and mark the current test (scope?) as SKIPPED.
    """
    pass


def skip_notebook(reason: str):
    """
    Shorthand function to send "skip notebook" signal.
    """
    raise SkipNotebook(reason)

In this spirit, async_execute_cell would able to be able to handle the CellExecutionError, and then dispatch to pytest.skip() based on catching the SkipNotebook runtime exception. It is not much different, but will save us from signaling the cause through the string suffix of the limited pytest.{skip,exit} functions, at the same time opening the door for other kinds of signals.

It feels like we are either telling a very esoteric story, or that we alternatively may be reinventing the wheel? ✨

@MichaelTiemannOSC
Copy link

I came here with precisely the same problem, and I'm having absolutely no luck skipping tests (using a variety of work-arounds). I tried the documented method of editing the notebooks metadata, adding:

  "nbreg": {
   "skip": true,
   "skip_reason": "Not ready for testing."
  }

to the notebook's metadata, and the NBRegressionFixture.check method happily ignores that. It seems that Jupyter and pytest live not only on different planets, but occupy different galaxies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants