Skip to content

Fix flaky backend tests that build a bare TestClient - #13824

Open
hysts wants to merge 1 commit into
mainfrom
fix/flaky-history-recording-tests
Open

Fix flaky backend tests that build a bare TestClient#13824
hysts wants to merge 1 commit into
mainfrom
fix/flaky-history-recording-tests

Conversation

@hysts

@hysts hysts commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Description

test/test_history.py's two recording tests and test/test_queueing.py::test_heartbeat_task_cancelled_after_stream_completes fail intermittently. All three drive the app through a bare TestClient(app), and starlette only keeps a single event loop across requests when the client is entered as a context manager. Without the with block each request gets its own blocking portal, and that loop closes as soon as the response is complete.

schedule_record_run files the record with asyncio.get_running_loop().create_task(...) and record_run then awaits anyio.to_thread.run_sync(...), so when the offload has not come back in time the task is left pending on a loop that will never run again and the record is never written. The tests then sit out the full 30 second _wait_for and report "no record was written". The heartbeat task in the queueing test is stranded the same way, which is why it ends up neither cancelled() nor done(). Under uvicorn the loop outlives the request, so this only ever affected the tests.

The fix is to enter the clients as context managers so one portal spans the whole test. Four places, nine lines, no library change.

Measured locally with CI's own command, pytest -n auto -m "not flaky and not serial":

before after
test/test_history.py alone 4 of 5 runs failed 8 of 8 clean
whole backend suite 4 of 5 runs failed 3 of 3 clean

The timing is the part worth keeping. Every failing run took 37 to 43 seconds, because it waited out the 30 second poll, and every run after the change took 7 to 17 seconds without entering the wait at all. That is what says the race is gone rather than just less frequent.

One caveat on evidence, since the three tests are not equally covered by the above. The two test_history.py tests reproduce locally and the numbers are theirs. test_heartbeat_task_cancelled_after_stream_completes does not reproduce locally, since it passes in isolation with or without the change; it is included because it was observed failing on CI and builds its client the same way. The change to it is safe either way, but it rests on that rather than on a measurement.

Closes: #13823

AI Disclosure

We encourage the use of AI tooling in creating PRs, but the any non-trivial use of AI needs be disclosed. E.g. if you used Claude to write a first draft, you should mention that. Trivial tab-completion doesn't need to be disclosed. You should self-review all PRs, especially if they were generated with AI.

  • I used AI to... I used AI to investigate the root cause and implement the fix.
  • I did not use AI

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Testing and Formatting Your Code

  1. PRs will only be merged if tests pass on CI. We recommend at least running the backend tests locally, please set up your Gradio environment locally and run the backed tests: bash scripts/run_backend_tests.sh

  2. Please run these bash scripts to automatically format your code: bash scripts/format_backend.sh, and (if you made any changes to non-Python files) bash scripts/format_frontend.sh

Three tests build a bare TestClient(app). Starlette only keeps one
event loop across requests when the client is entered as a context
manager; without the with block each request gets its own blocking
portal, and that loop closes as soon as the response is complete.

schedule_record_run files the record with create_task and record_run
then awaits anyio.to_thread.run_sync, so a write that has not come
back by the time the response is done is left pending on a loop that
will never run again. The tests then sit out the full 30 second
_wait_for and report "no record was written". The heartbeat task in
test_queueing is stranded the same way, ending up neither cancelled
nor done.

Entering the clients as context managers keeps one portal alive for
the whole test. Measured with CI's command, pytest -n auto -m "not
flaky and not serial": test_history.py alone went from 4 of 5 runs
failing to 5 of 5 clean, and the whole backend suite from 4 of 5
failing to 3 of 3 clean. Every failing run took 37 to 43 seconds
because it waited out the poll; every run after the change took 7 to
17 seconds without entering the wait at all.
@hysts hysts self-assigned this Sep 5, 2026
@hysts
hysts requested a lite review from Copilot September 5, 2026 02:05
@gradio-pr-bot

gradio-pr-bot commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

🪼 branch checks and previews

Name Status URL
🦄 Changes detected! Details

@gradio-pr-bot

Copy link
Copy Markdown
Collaborator

🦄 no changes detected

This Pull Request does not include changes to any packages.

__No changes detected. __



Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can create the changelog file directly.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a targeted, low-risk test-only fix that correctly aligns TestClient usage with Starlette’s expected lifecycle to eliminate the described race.

Pull request overview

This PR reduces backend test flakiness by ensuring Starlette’s TestClient is used as a context manager so a single event loop/portal is preserved across multiple requests within each test (preventing background tasks from being stranded on a closed loop).

Changes:

  • Update test/test_history.py fixtures/tests to create TestClient(app) via with TestClient(app) as client and yield the entered client.
  • Update test/test_queueing.py to enter TestClient(app) in the same with statement as the asyncio.create_task patch.
File summaries
File Description
test/test_queueing.py Enters TestClient as a context manager to keep the portal/event loop alive across requests in the heartbeat test.
test/test_history.py Enters TestClient as a context manager in recording-related fixtures/tests to prevent background recording tasks from being orphaned.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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 this pull request may close these issues.

Flaky backend tests: a bare TestClient closes the event loop before the background task finishes

3 participants