test(ui): isolate OldTeams delete-warning tests from leaked mock#30871
Conversation
The deprecated OldTeams component takes only accessToken, userID, userRole and premiumUser; it ignores the teams prop these tests passed and instead populates its table from the mocked teamListCall. The delete-warning block never set teamListCall, and vi.clearAllMocks clears call history but not implementations, so the table rendered the "Legacy Team" (keys.length 2) left behind by the previous block's last test. Both delete tests therefore ran against that leaked team: the keys-present case passed only because the leaked count happened to be 2, and the no-keys case rendered the same warning it asserted should be absent, so it failed. Seed the team through the channel the component actually reads (teamListCall) and drop the props it never consumes, so each test renders exactly the team it declares. The keys-present case now uses a distinctive count so it can no longer pass on a coincidental leak
Greptile SummaryFixes a deterministic test failure in the
Confidence Score: 5/5Test-only change with no production code touched; corrects mock isolation so both delete-warning tests accurately exercise the component's real data path. The fix addresses the root cause precisely: the component reads its team list from teamListCall, not from props, so seeding via that mock is the correct approach. Using a distinctive count (5) rather than 2 closes the coincidental-pass loophole. No assertions are weakened and no production behaviour is altered. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/OldTeams.test.tsx | Test isolation fix: openDeleteModal now seeds via teamListCall mock instead of a teams prop the component ignores; keys-present assertion upgraded to count 5 to rule out coincidental matches from prior test state. |
Reviews (2): Last reviewed commit: "test(ui): isolate OldTeams delete-warnin..." | Re-trigger Greptile
|
bugbot run |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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 245660d. Configure here.
yuneng-berri
deleted the
litellm_fix_oldteams_delete_warning_test_isolation
branch
…riAI#30871) The deprecated OldTeams component takes only accessToken, userID, userRole and premiumUser; it ignores the teams prop these tests passed and instead populates its table from the mocked teamListCall. The delete-warning block never set teamListCall, and vi.clearAllMocks clears call history but not implementations, so the table rendered the "Legacy Team" (keys.length 2) left behind by the previous block's last test. Both delete tests therefore ran against that leaked team: the keys-present case passed only because the leaked count happened to be 2, and the no-keys case rendered the same warning it asserted should be absent, so it failed. Seed the team through the channel the component actually reads (teamListCall) and drop the props it never consumes, so each test renders exactly the team it declares. The keys-present case now uses a distinctive count so it can no longer pass on a coincidental leak
2 participants
Relevant issues
ui_unit_testsis red onlitellm_internal_stagingbecause of one deterministic failure inOldTeams.test.tsx("still warns about model deletion in the confirmation message when the team has no keys"). This fixes thatLinear ticket
N/A
Pre-Submission checklist
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer review (Greptile returned 5/5, safe to merge)Root cause
The deprecated
OldTeamscomponent accepts onlyaccessToken,userID,userRoleandpremiumUser. It ignores theteamsprop that the delete-warning tests were passing and instead populates its table from the mockedteamListCall. That block never setteamListCall, andvi.clearAllMocks()clears call history but not implementations, so the table rendered the "Legacy Team" (keys.length2) left behind by the previous block's last test ("falls back to keys.length when keys_count is absent").Both delete tests therefore ran against that leaked team. The keys-present case passed only because the leaked count happened to be 2, and the no-keys case rendered the very "Warning: This team has 2 keys" message it asserted should be absent, so it failed
Fix
Seed the team through the channel the component actually reads (
teamListCall) and drop the props it never consumes, so each test renders exactly the team it declares. The keys-present case now uses a distinctive count (5) so it can no longer pass on a coincidental leakScreenshots / Proof of Fix
This is a frontend test-isolation fix, so the meaningful proof is the suite going from red to green on the clean
litellm_internal_stagingbase, run fromui/litellm-dashboard:Mutation checks confirming the tests still bite: forcing the component to always render the warning makes the no-keys test fail, and reverting the harness seeding (while keeping count 5) makes both delete tests fail because they render the leaked count-2 team
Type
✅ Test
Changes
ui/litellm-dashboard/src/components/OldTeams.test.tsx: drive the team data via theteamListCallmock thatOldTeamsreads, stop passing props the component ignores, and use a distinctive key count in the keys-present caseGenerated by Claude Code
Note
Low Risk
Test-only changes in OldTeams.test.tsx with no production code impact.
Overview
Fixes flaky/failing OldTeams delete-modal tests by seeding team data through the
teamListCallmock the component actually uses, instead of ateamsprop it ignores.The
openDeleteModalhelper now mocksteamListCallper test and rendersOldTeamswith onlyaccessToken,userID, anduserRole. The keys-present case assertskeys_count: 5so a leaked “2 keys” team from an earlier test can’t make the assertion pass by accident.Reviewed by Cursor Bugbot for commit 245660d. Bugbot is set up for automated code reviews on this repo. Configure here.