Add method = both: 1. signal + 2. thread
#89
Conversation
The new method aims at combining the advantages of `method = thread` (reliability) and `method = signal` (gracefulness, ability to continue). A new option `kill_delay` is added to configure how much time the test is granted to handle the timeout thrown by the signalling method. After that time has expired, the `thread` method is used. Two new tests ensure that the new method works as expected, existing tests have been parametrised to include the new method. The README now contains a small section documenting the method and its `kill_delay` configuration option.
| 'both' tries to gracefully time out a test, after kill_delay seconds | ||
| a harsh kill is used to reliably stop the test. |
There was a problem hiding this comment.
I think it's better to describe this in terms of the signal and thread terminology used in this help message already rather than use "gracefully" and "harsh". Maybe something more like
| 'both' tries to gracefully time out a test, after kill_delay seconds | |
| a harsh kill is used to reliably stop the test. | |
| 'both' uses 'signal' by default, but if this fails to time out the test falls back to 'thread' after kill_delay seconds. |
(but at least properly formatted 😄
| the soft *signal* attempt, so you might want to switch to *thread*. | ||
| Similar to ``PYTEST_TIMEOUT``, you can specify the kill delay with the | ||
| ``PYTEST_KILL_DELAY`` environment variable. | ||
|
|
There was a problem hiding this comment.
I think it would be good to also describe the tradeoffs a little: you carry the overheads of spawning a thread for each test but have the chance of not killing the entire process on each timeout while still having the reliability of ensuring you always get a working timeout.
(maybe there's better ways to phrase that)
| time.sleep(2) | ||
|
|
||
| # so that the signal method does not succeed | ||
| signal.signal(signal.SIGALRM, handler) | ||
| time.sleep(2) |
There was a problem hiding this comment.
heh, i had to double-take to be sure this wasn't a little racy. I think it would be clearer if the new handler just does a pass and the sleep of the test clearly covers > 2s right away, say 3s.
You can even turn the new handler in a lambda if you like lambda signum, frame: pass is probably fine.
|
Make sure that this doesn't re-introduce #8. |
|
@flub: I get all the points you mentioned and will fix them, just didn't want to respond with the same text three times... |
|
I see it did not get to |
|
closing as this seems stalled |
STATUS: rather drafty, feedback welcome. Will be modified soon.
The new method aims at combining the advantages of
method = thread(reliability) andmethod = signal(gracefulness, ability to continue).A new option
kill_delayis added to configure how much time the test is granted to handle the timeout thrown by the signalling method. After that time has expired, thethreadmethod is used.Two new tests ensure that the new method works as expected, existing tests have been parameterized to include the new method.
The README now contains a small section documenting the method and its
kill_delayconfiguration option.Partially related (motivated by): #87.
Also related (it seems??): #88.