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

special handling for unawaited coroutine warnings #86

Open
njsmith opened this issue Oct 14, 2019 · 0 comments
Open

special handling for unawaited coroutine warnings #86

njsmith opened this issue Oct 14, 2019 · 0 comments

Comments

@njsmith
Copy link
Member

njsmith commented Oct 14, 2019

In chat today, @catern was talking about how they got burned badly by forgetting an await, and was planning to make their test suite capture these warnings and fail any test that issued them. (For more background on why this is a difficult situation to handle well, see python-trio/trio#79).

Maybe pytest-trio should do this by default, on a best-effort basis? Pytest is already capturing warnings and attaching them to each test's output. There are some limitations:

  1. Sometimes the unawaited coroutine warning shows up later; possibly in a different, unrelated test. We could reduce the chances of this by invoking the GC after each test. Maybe we don't want to invoke the GC after every single test we run, though? It might slow things down. How much? I guess the worst case would be test suites with lots and lots of tests, to make the live set as made as possible? (Would it make sense to use different policies on CPython and PyPy?)

  2. Pytest gives a lot of options for configuring its warning capture, or disabling it altogether. We can't go breaking this. So maybe we'd just need to be careful to gracefully disable this feature whenever pytest's warning capture is in a non-default mode?

  3. In many cases, a test will crash and the root cause will be a missing await, but the proximal cause is something else and the resulting error is super confusing. In these cases, it would be nice if this feature could help users pinpoint the root cause. But this is tricky, because:

  • sometimes you don't get a warning in these cases, because the crash traceback captures the test's stack frames, so the coroutine __del__ doesn't run, so it doesn't issue the warning.

  • sometimes you do get a warning, but this is just a symptom, not the root cause. In particular, this tends to happen for certain kinds of violations of Trio's internal invariants that cause the scheduler to error out. Maybe a simple heuristic would be good enough though, like: if the test crashed with TrioInternalError, then don't worry about unawaited coroutines?

Again, this would be a best effort feature, so we don't have to solve all these problems perfectly from the start.

catern added a commit to catern/rsyscall that referenced this issue Feb 7, 2022
Forgetting to await a coroutine may make a test do essentially
nothing.  We want to make sure that doesn't happen.

Probably raise_unraisables should be done automatically by
unittest.TestCase.

Ideally, forgetting an await would be an error at the Python level,
see also python-trio/pytest-trio#86
catern added a commit to catern/rsyscall that referenced this issue Feb 7, 2022
Forgetting to await a coroutine may make a test do essentially
nothing.  We want to make sure that doesn't happen.

Probably raise_unraisables should be done automatically by
unittest.TestCase.

Ideally, forgetting an await would be an error at the Python level,
see also python-trio/pytest-trio#86
catern added a commit to catern/rsyscall that referenced this issue Feb 7, 2022
Forgetting to await a coroutine may make a test do essentially
nothing.  We want to make sure that doesn't happen.

Probably raise_unraisables should be done automatically by
unittest.TestCase.

Ideally, forgetting an await would be an error at the Python level,
see also python-trio/pytest-trio#86
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

No branches or pull requests

1 participant