This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: unittest ignores the first ctrl-c when it shouldn't
Type: behavior Stage:
Components: Library (Lib) Versions:
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: mgedmin, michael.foord, rbcollins
Priority: normal Keywords: patch

Created on 2015-12-18 08:48 by mgedmin, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
dont-ignore-first-ctrl-c.patch mgedmin, 2015-12-18 08:48 review
dont-ignore-first-ctrl-c-with-tests.patch mgedmin, 2015-12-22 08:10 v2 review
Messages (6)
msg256656 - (view) Author: Marius Gedminas (mgedmin) * Date: 2015-12-18 08:48
unittest.signals._InterruptHandler is very nice: on first ^C it tells any registered unittest Result instances to gracefully stop the test run, and on any subsequent ^C it defers to the default interrupt handler, which immediately terminates the test run (possibly without giving it a chance to output summary information).

This doesn't work very well when a test runner forgets to register the Result instance: the first ^C is then quietly ignored.  (See https://github.com/django-nose/django-nose/issues/252 for an example.)

Suggestion: if there are no registered results in the unittest.signals._results dict, don't silently ignore the first ^C.  Instead, immediately defer to the saved interrupt handler.

Since code is often clearer than words, I'm attaching a patch.
msg256800 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2015-12-21 11:37
Suggested behaviour sounds good, patch looks sensible. Is it possible to add a test too?
msg256830 - (view) Author: Marius Gedminas (mgedmin) * Date: 2015-12-22 08:02
I looked for any existing tests (by grepping for 'signals' and 'import' on the same line), didn't find any (because my assumption that 'signals' would have to be explicitly imported was wrong).

Wrote some new tests, which made me also make some further changes to unittest/signals.py: https://gist.github.com/mgedmin/a91872054884dbaaa344

And that's how I discovered that there's an existing test suite for SIGINT handling, in test_break.py, and that it fails after my changes.

Stay tuned.
msg256832 - (view) Author: Marius Gedminas (mgedmin) * Date: 2015-12-22 08:10
Here's an updated patch with tests
msg256833 - (view) Author: Marius Gedminas (mgedmin) * Date: 2015-12-22 08:19
Somewhat unrelated nitpick: I'm not very happy that _InterruptHandler doesn't return early after calling the original handler.

It's all very well, if the original handler raises or does nothing at all, but if it's a user-installed handler that does something (e.g. print a stack trace to sys.stderr), then it might get called twice (or three times, after my patch).

AFAIU the situation would have to be pretty contrived:

- there's a user-provided SIGINT handler
- unittest installs its own on top
- one of the tests (or some test runner plugin) overrides the SIGINT handler again
- user presses Ctrl-C
- the current SIGINT handler delegates to the unittest _InterruptHandler
- _InterruptHandler notices it's not the current handler and delegates to the original one
- the original handler doesn't raise
- _InterruptHandler.__call__ doesn't return early and goes on to perform the "if called check"
- if this is a second Ctrl-C, the original handler will get called a second time for the same Ctrl-C event
- if there are no registered results, my new code will cause the original handler to get called again (a third, or a 2nd time for the same Ctrl-C event)

Is there a good reason not to add early returns after defering to the original handler?  Maybe it's desired that ^C should stop the test suite even if a test overrides the SIGINT handler but delegates back to the unittest one?
msg261706 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2016-03-14 01:27
I'd rather make this super simple: just terminate the test run immediately. We can catch KeyBoardInterrupt in the UI layer to still permit outputting summary information. That removes one more source of global state that makes testing fragile.
History
Date User Action Args
2022-04-11 14:58:25adminsetgithub: 70088
2016-03-14 01:27:23rbcollinssetmessages: + msg261706
2015-12-22 08:19:32mgedminsetmessages: + msg256833
2015-12-22 08:10:24mgedminsetfiles: + dont-ignore-first-ctrl-c-with-tests.patch

messages: + msg256832
2015-12-22 08:02:01mgedminsetmessages: + msg256830
2015-12-21 11:37:41michael.foordsetmessages: + msg256800
2015-12-18 16:29:00r.david.murraysetnosy: + rbcollins, michael.foord
2015-12-18 08:48:29mgedmincreate