Option to stop executing notebook at first failure?

[ceball]

Summary: What do you think about adding --maxfail-per-notebook=n, to allow stopping executing a notebook after its first failure?

When using --nbval-lax, I really only want to know if the notebook executes successfully or not. If a cell fails (i.e. results in an exception), I would expect subsequent ones to fail or be meaningless - so I only want to learn about the first failure. I'd like there to be an option like "stop on first exception per notebook"?

I see there might be cases where you want to know things like, "how much of my notebook executes successfully?", or "do I have failures in my notebook that don't affect the rest of the notebook?", but that seems less common than just wanting to know if the notebook executes ok.

If something like --nbval-run were added (i.e. a "no output checking" mode; discussion in #141), I would also propose making stop on first error per notebook be the default behavior for that option.

I think it's also possible if using --nbval (i.e. doing output checking) that you might want to stop on the first output match failure per notebook, sometimes. E.g. you can imagine having a bunch of long running notebooks, and having cases where if one output is wrong, all subsequent outputs will be wrong or meaningless - so you don't want to keep executing past the first failure. So, maybe the option I'm proposing is more like "stop on first failure" and applicable to all uses of nbval, rather than just stopping on first exception when running without output checks?

I've considered whether I can do what I want with pytest, but I don't think so. pytest has options to stop executing after a number of failures, but as far as I'm aware, such options cannot be applied/targeted as I need. If I test 100 notebooks, I want to try to run each independently, only stopping an individual notebook's tests at the first failure in that notebook, rather than all tests. (Maybe I should instead open a feature request on pytest to support --maxfail-per-class?)

I would implement this in nbval if it makes sense to others.

[vidartf]

One possibility is to follow the example of what we do for timeouts:

nbval/nbval/plugin.py

Lines 530 to 536 in 3597e0b

	if self.parent.timed_out:
	# xfail(condition, reason=None, run=True, raises=None, strict=False)
	xfail_mark = pytest.mark.xfail(
	True,
	reason='Previous cell timed out, expected cell to fail'
	)
	self.add_marker(xfail_mark)

We could then have an option --[nbval-]on-cell-failure=[continue,error,expectFail] or something along those lines (names are hard). Then if a cell fails, we can modify the following cells accordingly:

• continue would just do as we currently do
• error would cause following cells to error (vs fail)
• expectFail would mark all following cells as expected to fail (with strict=False)

[takluyver]

I think the default should probably be to skip or xfail the remainder of the notebook - as you say, once one cell has failed, running the rest of the notebook probably doesn't make sense, and the extra errors make it harder to see the real problem. I'd lean towards skip, to save time executing them. I'm not too concerned about changing this, because it can make tests pass that would otherwise fail.

Of course this only applies if there's an error executing a cell. If output validation fails, it can still carry on checking subsequent cells.

If an option is needed, I think I prefer a simple switch rather than a numeric thing. I can't see that you'd ever set --maxfail-per-notebook to anything other than 1 or infinity. But I also can't see a use for making following cells error - the original error will cause a test failure anyway, so why generate more errors?

So maybe the option should be a simple flag --nbval-continue-on-error? (Remind you of anything?)

Or perhaps there should be a way to control it per notebook?

[ocefpaf]

👍 for --nbval-continue-on-error. IMO the default should be False but that would break the current code and maybe we are already used to continuing and, to stop, we will need to set the flag.

@takluyver I'm not familiar with the code base here but if you can give me some guidance I can try a PR.