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

Replace async-timeout with pytest-timeout and builtin asyncio routines #880

Merged
merged 2 commits into from
Dec 25, 2021

Conversation

nolar
Copy link
Owner

@nolar nolar commented Dec 25, 2021

async-timeout was used in tests for 2 purposes:

  • Extra safety against hanging the test-run if some tests are designed improperly. This function is now taken by pytest-timeout in CI runs.
  • As a part of test design to limit the expectedly infinite or long sleep.

async-timeout was convenient to put on the same line as the pytest.raises(asyncio.TimeoutError) that it causes. Since async-timeout stopped supporting sync context manager protocol, it now requires a separate async with and an additional level of indenting — so, it became less useful, or not useful at all. This job is better done by the native asyncio.wait_for(…, timeout=…).

This change will also simplify the tests in the coming refactoring with the fake (simulated) time of event loops.

@nolar nolar added the automation CI/CD: testing, linting, releasing automatically label Dec 25, 2021
@nolar nolar enabled auto-merge December 25, 2021 22:02
Normally, individual tests should not take longer than 1 second, preferably closer to 0 seconds. Some selected tests, however, are slow by their nature (e.g. ngrok or kopf-runner) — so they take around 1-2 seconds. But this limit is only applicable to CI — not to local dev-mode runs, where breakpoints and infinite pauses are needed.

This change should prevent the test-runs from being stuck at one single test if the test is designed improperly or because of the environment issues. This removes the need to keep the timing safeguards inside the tests, which complicates the tests themselves (e.g. with `async-timeout`) — unless such safe-guards are a part of the test design.

Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
`async-timeout` was used in tests for 2 purposes:

* Extra safety against hanging the test-run if some tests are designed improperly. This function is now taken by `pytest-timeout` (see the previous commit).
* As a part of test design to limit the expectedly infinite or long sleep.

`async-timeout` was convenient to put on the same line as the `pytest.raises(asyncio.TimeoutError)` that it causes. Since `async-timeout` stopped supporting sync context manager protocol, it now requires a separate `async with` and an additional level of indenting — so, it became less useful, or not useful at all. This job is better done by the native `asyncio.wait_for(…, timeout=…)`.

This change will also simplify the tests in the coming refactoring with the fake (simulated) time of event loops.

Signed-off-by: Sergey Vasilyev <nolar@nolar.info>
@nolar nolar merged commit 06d0094 into main Dec 25, 2021
@nolar nolar deleted the get-rid-of-async-timeout branch December 25, 2021 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation CI/CD: testing, linting, releasing automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant