Skip to content

test: wait on conditions instead of wall-clock in guardrail parallelism and scope-option tests - #45571

Merged
mateo-berri merged 2 commits into
mainfrom
litellm_deflake_20261009
Oct 9, 2026
Merged

mateo-berri merged 2 commits into
mainfrom
litellm_deflake_20261009

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • Two tests flaked on CI runners under load in the last 24h
  • The guardrail form test queried a select option before it rendered
  • The guardrail parallelism test asserted a 0.2s wall-clock bound

How it solves it:

  • Await the expected scope option with findByRole before checking absences
  • Replace sleeps plus the timing bound with an asyncio.Event barrier

CI evidence, mechanism, and fix

Run date 2026-10-09 (window 2026-10-08 09:19 UTC to 2026-10-09 09:19 UTC)

add_guardrail_form.characterization.test.tsx > hides directional scope choices for providers that do not support them, 15 failures in the window, every one on attempt 1 of the UI Unit Tests workflow, at SHAs where the same test passed on neighbouring commits of the same branch (main included). Failure text: Unable to find an accessible element with the role "option" and name "Both (request and response)". Examples on main: run 37847701422 attempt 1, run 37874103404 attempt 1, run 37876710808 attempt 1. Others: 37815547855, 37847959230, 37849567681, 37852112339, 37857584458, 37857661078, 37858777401, 37859690767, 37863670485, 37867437929, 37872367145, 37903646845, all attempt 1. It also failed locally 4 of 20 file runs on unedited main with the box under load

Mechanism: async render race. The scope dropdown renders its options after the click settles, and the test used the synchronous getByRole for the one option that must be present. The two queryByRole(...).not.toBeInTheDocument() checks ran first, so on a slow tick they also passed vacuously against a dropdown that had not opened yet

Fix: await screen.findByRole("option", { name: "Both (request and response)" }) first, then the two absence checks, which now run against an open dropdown. Mutating supportsDirectionalLoggingOnlyScope to always return true makes the test fail on the absence check

tests/unit/proxy/test_proxy_utils.py::test_during_call_hook_parallel_execution, 5 failures in the window, all attempt 1 of the Unit Tests workflow (proxy-infra-root shard), each on a different PR whose next run passed it: 37799269742, 37824913943, 37875589831, 37881579012, 37906512189. Failure text: Parallel execution took 1.10s, expected < 0.2s

Mechanism: wall-clock dependence. Three guardrails each asyncio.sleep(0.1) and the test asserted the whole during_call_hook finished in under 0.2s, which a loaded xdist worker misses even though the hooks still ran concurrently

Fix: each guardrail records its start, the last one to start sets an asyncio.Event, and every guardrail waits on that event (bounded by wait_for(..., timeout=5)) before recording its end. Concurrent execution releases the barrier immediately, sequential execution can never reach it, so the test checks parallelism by ordering and not by duration. Replacing asyncio.gather(*guardrail_tasks) in ProxyLogging.during_call_hook with sequential awaits makes the test fail with TimeoutError

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • The handful of test files covering my change pass locally, e.g. uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*, make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more
  • My PR passes all required CI/CD checks (e.g., lint, schema.d.ts sync check, etc.)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Type

✅ Test

Caveats (if any)

Low

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

Link to Devin session: https://app.devin.ai/sessions/1766c3e04dc7459688fb9df5f70f35b9
Open in Devin Desktop: https://app.devin.ai/desktop/session/1766c3e04dc7459688fb9df5f70f35b9?variant=devin


Note

Low Risk
Test-only changes with no production code; risk is limited to test behavior and CI signal quality.

Overview
Deflakes two CI tests by replacing timing- and sync-query assumptions with condition-based waits.

In test_during_call_hook_parallel_execution, guardrails no longer use fixed asyncio.sleep plus a < 0.2s total-time check. They coordinate with an asyncio.Event so each hook blocks until all three have started; parallel runs pass quickly, sequential runs time out. The test still asserts that every guardrail starts before any ends.

In the guardrail form characterization test for XecGuard logging-only scope, the test now awaits the "Both (request and response)" option with findByRole before asserting that input/output-only options are absent, so absence checks run against a fully opened dropdown instead of racing async render.

Reviewed by Cursor Bugbot for commit 6a1c8dd. Bugbot is set up for automated code reviews on this repo. Configure here.

…sm and scope-option tests

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] This test-only PR appears safe to merge

Summary

This PR makes two tests wait for the conditions they check instead of relying on timing

  • The parallelism test waits until all three guardrails have started.
  • The scope test waits for its option before checking hidden choices.

Reviews (2) · Last reviewed commit: "test: annotate the guardrail barrier loc..." · Reviewed by Greptile

Comment thread tests/unit/proxy/test_proxy_utils.py Outdated
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@mateo-berri

Copy link
Copy Markdown
Contributor

@greptileai

@mateo-berri

Copy link
Copy Markdown
Contributor

bugbot run

@cursor cursor Bot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 6a1c8dd. Configure here.

@mateo-berri mateo-berri 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.

LGTM

@mateo-berri
mateo-berri merged commit dd31339 into main Oct 9, 2026
88 checks passed
@mateo-berri
mateo-berri deleted the litellm_deflake_20261009 branch October 9, 2026 10:41
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.

1 participant