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

Add test timeout support #53

Open
njsmith opened this issue Jul 24, 2018 · 2 comments · May be fixed by #136
Open

Add test timeout support #53

njsmith opened this issue Jul 24, 2018 · 2 comments · May be fixed by #136

Comments

@njsmith
Copy link
Member

njsmith commented Jul 24, 2018

This will need some help from Trio – see python-trio/trio#168

@njsmith
Copy link
Member Author

njsmith commented Jul 26, 2018

Actually.... this doesn't need help from Trio, if we're clever. It's not the most elegant thing, but we can start a timer thread running, and then if the timeout expires it can call back into Trio. Of course we'll also want to be able to kill it early, so something like, a thread blocked in select would work.

def timeout_thread(wake_sock, timeout, trio_token, cancel_scope):
    readable, _, _ = select.select([wake_sock], [], [], timeout)
    if not readable:
        # we were woken by the timeout expiring
        try:
            trio_token.run_sync_soon(cancel_scope.cancel)
        except trio.RunFinishedError:
            pass

async with trio.open_nursery() as nursery:
    a, b = socket.socketpair()  # blocking sockets
    trio_token = trio.hazmat.current_trio_token()
    try:
        with trio.open_cancel_scope() as timeout_cancel_scope:
            nursery.start_soon(trio.run_sync_in_worker_thread, timeout_thread, b, timeout, trio_token, timeout_cancel_scope, limiter=UNLIMITED)
            # ... do actual test here ...
    finally:
        a.send(b"x")
        if timeout_cancel_scope.cancelled_caught:
            raise ...

This does require that the Trio scheduler be functioning properly, but so would a more intrusive version baked into the Trio scheduler. This also has the advantage that by being in a third-party library we have a lot of flexibility to adjust the response to the timeout however we want – e.g. instead of just cancelling the test, we could have it capture and print a snapshot of the task tree with stacktraces. Or try cancelling after X seconds, and then if the test is still running after another Y seconds (i.e. it's ignoring the cancellation), use harsher methods to disable pytest's output capturing, dump debugging information to the screen, and then call os._exit() to crash the process.

@njsmith
Copy link
Member Author

njsmith commented Jun 7, 2019

In python-trio/trio#168 I suggested that we might want to have two separate timeouts – an idle timeout and a global timeout:

What twisted uses is a simple global timeout for the whole test, after which I assume it just stops executing, abandoning any callback chains mid-flight. For many purposes an idle-time timeout makes more sense ("if all tasks have been asleep for X seconds, cancel the test") - in particular, one can set a pretty aggressive timeout for this so that frozen tests fail fast, but big complicated tests that just do a lot of stuff won't trigger it. However, we probably also want a single cumulative timeout ("if the test started X seconds ago, cancel it"), to catch cases like the ssl test that's mentioned in bpo-30437, where a misbehaving test was constantly sending data in an infinite loop, so it never went idle but would have responded to an absolute timeout.

On further consideration, I think a deadlock detector like in python-trio/trio#1085 would be a better solution for the "idle timeout" use cases, so maybe pytest-trio should just focus on providing a global timeout.

VincentVanlaer added a commit to VincentVanlaer/pytest-trio that referenced this issue Sep 3, 2023
Whenever the trio_timeout option is enabled, this plugin will hook into
requests from pytest-timeout to set a timeout. It will then start a
thread in the background that, after the timeout has reached, will
inject a system task in the test loop. This system task will collect
stacktraces for all tasks and raise an exception that will terminate the
test. The timeout thread is reused for other tests as well to not incur
a startup cost for every test.

Since this feature integrates with pytest-timeout, it also honors things
like whether a debugger is attached or not.

Drawbacks:

- Ideally, whether trio does timeouts should not be a global option, but
  would be better suited for the timeout-method in pytest-timeout. This
  would require a change in pytest-timeout to let plugins register other
  timeout methods.
- This method requires a functioning loop.

Fixes python-trio#53
@VincentVanlaer VincentVanlaer linked a pull request Sep 3, 2023 that will close this issue
VincentVanlaer added a commit to VincentVanlaer/pytest-trio that referenced this issue Sep 3, 2023
Whenever the trio_timeout option is enabled, this plugin will hook into
requests from pytest-timeout to set a timeout. It will then start a
thread in the background that, after the timeout has reached, will
inject a system task in the test loop. This system task will collect
stacktraces for all tasks and raise an exception that will terminate the
test. The timeout thread is reused for other tests as well to not incur
a startup cost for every test.

Since this feature integrates with pytest-timeout, it also honors things
like whether a debugger is attached or not.

Drawbacks:

- Ideally, whether trio does timeouts should not be a global option, but
  would be better suited for the timeout-method in pytest-timeout. This
  would require a change in pytest-timeout to let plugins register other
  timeout methods.
- This method requires a functioning loop.

Fixes python-trio#53
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

Successfully merging a pull request may close this issue.

1 participant