Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Dec 13, 2025

Fixes #19762

When prefect_test_harness is used in an async context, the drain_all() and drain() methods return coroutines that are never awaited, causing RuntimeWarning: coroutine 'wait' was never awaited.

Root cause: The drain() and drain_all() methods in _QueueServiceBase are "dual-mode" - they return awaitables when called from an async context (when get_running_loop() is not None). Since prefect_test_harness is a sync context manager, these returned coroutines were being dropped without being awaited.

Solution: Use a wrapper coroutine passed to run_coro_as_sync that calls the public drain() and drain_all() methods and awaits them if they return awaitables. This ensures the awaitable is created and awaited on the same loop, avoiding cross-loop issues while using public APIs.

Key changes:

  • Add a drain_workers() async helper that calls APILogWorker.instance().drain() and EventsWorker.drain_all()
  • Check if results are awaitable with inspect.isawaitable() and await them if so
  • Execute the wrapper coroutine via run_coro_as_sync() to handle both sync and async contexts
  • Add regression test that catches unawaited coroutines using PytestUnraisableExceptionWarning

Human review checklist:

  • Verify the wrapper coroutine approach correctly handles both sync and async contexts
  • Verify error handling for when APILogWorker hasn't been started
  • Verify the regression test properly catches unawaited coroutines

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

Link to Devin run: https://app.devin.ai/sessions/70901d7018934b0f9269d6e42042e4a3
Requested by: Nate Nowack ([email protected]) (@zzstoatzz)

…c contexts

Fixes #19762

When prefect_test_harness is used in an async context, the drain_all() and
drain() methods return coroutines that were never awaited, causing
RuntimeWarning: coroutine 'wait' was never awaited.

This fix uses the underlying _drain() method which returns a
concurrent.futures.Future that can be waited on synchronously, regardless
of whether there's a running event loop. This avoids creating unawaited
coroutines in the first place.

Changes:
- Use _drain() instead of drain()/drain_all() in prefect_test_harness cleanup
- Wait synchronously on the futures with concurrent.futures.wait()
- Copy EventsWorker instances before iterating to avoid dictionary mutation error
- Add regression test that fails if unawaited coroutines are created

Co-Authored-By: Nate Nowack <[email protected]>
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions github-actions bot added the bug Something isn't working label Dec 13, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 13, 2025

CodSpeed Performance Report

Merging #19770 will not alter performance

Comparing devin/1765652244-fix-test-harness-async-cleanup (d3141ff) with main (a4ae0c1)

Summary

✅ 2 untouched

@zzstoatzz zzstoatzz marked this pull request as ready for review December 15, 2025 17:59
devin-ai-integration bot and others added 2 commits December 15, 2025 18:22
…lic API usage

Address reviewer feedback to avoid using private _drain() methods.
Instead, use a wrapper coroutine that calls drain()/drain_all() and
awaits the result if it's awaitable. This ensures the awaitable is
created and awaited on the same loop, avoiding cross-loop issues.

Also move gc import to top of test file per project conventions.

Co-Authored-By: Nate Nowack <[email protected]>
@zzstoatzz zzstoatzz enabled auto-merge (squash) December 17, 2025 17:52
@zzstoatzz zzstoatzz disabled auto-merge December 17, 2025 18:05
@zzstoatzz zzstoatzz requested a review from desertaxle December 17, 2025 18:05
@zzstoatzz zzstoatzz merged commit 962e5a0 into main Dec 17, 2025
59 of 60 checks passed
@zzstoatzz zzstoatzz deleted the devin/1765652244-fix-test-harness-async-cleanup branch December 17, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prefect test harness has improper cleanup when run in an async context

3 participants