Skip to content

test(ui): settle the TeamSSOSettings save before the test returns - #41219

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
litellm_deflake_20260915
Open

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
litellm_deflake_20260915

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

  • TeamSSOSettings.test.tsx left a 100ms save timer running past teardown
  • In CI's reused forks that fired as ReferenceError: window is not defined
  • One unhandled error failed the whole UI unit job

How it solves it:

  • The save mock is a deferred promise the test resolves itself
  • The test waits for the component to leave edit mode before returning
  • Nothing async outlives the test, and the Cancel-disabled assertion stays

User Flow

Not applicable, this PR only changes a dashboard unit test

Relevant issues

Affected release

Linear ticket

Flake evidence, grouped by run date

2026-09-14

ui/litellm-dashboard/src/components/TeamSSOSettings.test.tsx, "should disable cancel button while saving", flaked once in the window

Run https://github.com/BerriAI/litellm/actions/runs/34869609876 (UI Unit Tests, head 9cd8829): attempt 1 failed in job https://github.com/BerriAI/litellm/actions/runs/34869609876/job/104061957175 with all 2582 tests passing and one unhandled error, ReferenceError: window is not defined at resolveUpdatePriority -> dispatchSetState -> handleSave src/components/TeamSSOSettings.tsx:162. Attempt 2 on the same SHA passed

Mechanism: async teardown race. The test mocked the save with setTimeout(..., 100), asserted that Cancel was disabled, and returned. CI runs the suite with --pool forks --maxWorkers=14, so the fork stays alive for the next file while the previous file's jsdom window is already gone. When the timer fired, handleSave continued into setValues/setIsEditing/setSaving and React's scheduler read window. A single-file local run does not reproduce this because the fork exits before the timer fires, so the fix rests on the CI stack trace plus the test structure (this is the only test in the file that returns with a pending timer)

Fix: Promise.withResolvers gives the test the handle to settle the save. It asserts Cancel is disabled while pending, resolves the save, then awaits the Edit Settings button, which only reappears after setIsEditing(false) runs. Mutating the component by removing disabled={saving} fails the first assertion, and removing setIsEditing(false) fails the second, so the test still fails when either behavior breaks

Candidates seen in the window but not fixed here

tests/proxy_behavior/auth/test_auth_object_prefetch.py::test_join_binds_the_membership_to_the_requested_team failed 87 times across runs 34901928062, 34911769309 and 34921376544 (all attempts), always on branch SHAs that predate f4e21430c0 (frozen cache clock in the prefetch tests) on main. Stale branch, the fix already landed upstream

test_file_reference_non_image_not_counted_as_image, test_video_plus_audio_does_not_double_bill_text, the Vertex embedding and Claude config cases, and test_ui_view_spend_logs_search_matches_any_id failed on every attempt of the runs they appeared in, on branches whose code changed the behavior under test. Reported as real breakages on those branches, not touched here

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/test_litellm/<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).

Screenshots / Proof of Fix

Test-only change with no user-facing behavior, so there is no live proxy or UI run to show. The CI evidence above is the before, and the ui-unit-tests job on this PR is the after

Type

✅ Test

Caveats (if any)

Low

  • The CI race did not reproduce locally, even with two files in one fork
  • The mechanism is pinned from the CI stack trace, not a local repro

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/08522e71689e42d9b9d5f25650594c5f
Open in Devin Desktop: https://app.devin.ai/desktop/session/08522e71689e42d9b9d5f25650594c5f?variant=devin


Note

Low Risk
Test-only change with no production code or runtime behavior impact.

Overview
Fixes a CI flake in TeamSSOSettings.test.tsx where the “disable Cancel while saving” case could finish while a mocked save was still pending.

The test no longer uses a setTimeout(100) save mock that could fire after jsdom teardown under Vitest’s fork pool. It now drives the update API with Promise.withResolvers, keeps the Cancel-disabled assertion while the promise is open, then resolves the save and waits for “Edit Settings” so the async save path fully settles before the test ends.

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

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration
devin-ai-integration Bot requested a review from a team September 15, 2026 10:01
@devin-ai-integration

Copy link
Copy Markdown
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, CI, and merge conflict monitoring

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes a teardown race from the TeamSSOSettings unit test

  • Replaces the pending timer with a deferred promise controlled by the test
  • Preserves the assertion that Cancel is disabled during saving
  • Waits for the component to leave edit mode after the save resolves

Confidence Score: 5/5

The test-only change appears safe to merge and resolves the pending asynchronous work without weakening the existing behavior check

The deferred save is supported by the configured Node and TypeScript toolchain, and the final awaited UI state keeps the save continuation within the test lifecycle

Important Files Changed

Filename Overview
ui/litellm-dashboard/src/components/TeamSSOSettings.test.tsx Makes the save lifecycle deterministic and ensures the asynchronous component update finishes before test teardown

Reviews (1): Last reviewed commit: "test(ui): settle the TeamSSOSettings sav..." | Re-trigger Greptile

@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 3c92161. 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

This branch has not been deployed

No deployments
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.

2 participants