From 1fb5c496c8f467b27587b3ed009c3da253e4f7c3 Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 24 Feb 2026 15:48:31 -0800 Subject: [PATCH] address greptile review feedback (greploop iteration 1) - Replace document.querySelector/querySelectorAll with screen.getByRole - Replace raw dispatchEvent with userEvent.selectOptions --- .../LatencyBasedConfiguration.test.tsx | 3 +-- .../ReliabilityRetriesSection.test.tsx | 3 +-- .../router_settings/RouterSettingsForm.test.tsx | 7 +++---- .../RoutingStrategySelector.test.tsx | 16 ++++++---------- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/ui/litellm-dashboard/src/components/router_settings/LatencyBasedConfiguration.test.tsx b/ui/litellm-dashboard/src/components/router_settings/LatencyBasedConfiguration.test.tsx index 0176be5ab40..39d9f8c881c 100644 --- a/ui/litellm-dashboard/src/components/router_settings/LatencyBasedConfiguration.test.tsx +++ b/ui/litellm-dashboard/src/components/router_settings/LatencyBasedConfiguration.test.tsx @@ -47,8 +47,7 @@ describe("LatencyBasedConfiguration", () => { const args = { ttl: { nested: true } }; render(); // HTML input type=text strips newlines, so check that the key/value appears - const input = document.querySelector('input[name="ttl"]') as HTMLInputElement; - expect(input).not.toBeNull(); + const input = screen.getByRole("textbox", { name: /ttl/i }) as HTMLInputElement; expect(input.value).toContain('"nested"'); expect(input.value).toContain('true'); }); diff --git a/ui/litellm-dashboard/src/components/router_settings/ReliabilityRetriesSection.test.tsx b/ui/litellm-dashboard/src/components/router_settings/ReliabilityRetriesSection.test.tsx index 0892d39fc29..101b09af0dc 100644 --- a/ui/litellm-dashboard/src/components/router_settings/ReliabilityRetriesSection.test.tsx +++ b/ui/litellm-dashboard/src/components/router_settings/ReliabilityRetriesSection.test.tsx @@ -67,8 +67,7 @@ describe("ReliabilityRetriesSection", () => { const settings = { retry_policy: { "rate-limited": 2 } }; render(); // HTML input type=text strips newlines, so check that the key/value appears - const input = document.querySelector('input[name="retry_policy"]') as HTMLInputElement; - expect(input).not.toBeNull(); + const input = screen.getByRole("textbox", { name: /retry_policy/i }) as HTMLInputElement; expect(input.value).toContain('"rate-limited"'); expect(input.value).toContain('2'); }); diff --git a/ui/litellm-dashboard/src/components/router_settings/RouterSettingsForm.test.tsx b/ui/litellm-dashboard/src/components/router_settings/RouterSettingsForm.test.tsx index 767820cd485..01f318b909a 100644 --- a/ui/litellm-dashboard/src/components/router_settings/RouterSettingsForm.test.tsx +++ b/ui/litellm-dashboard/src/components/router_settings/RouterSettingsForm.test.tsx @@ -97,8 +97,9 @@ describe("RouterSettingsForm", () => { expect(screen.getByText("Latency-Based Configuration")).toBeInTheDocument(); }); - it("should call onChange with the updated strategy when the selector changes", () => { + it("should call onChange with the updated strategy when the selector changes", async () => { const onChange = vi.fn(); + const user = userEvent.setup(); const props = { ...baseProps, onChange, @@ -106,9 +107,7 @@ describe("RouterSettingsForm", () => { }; render(); - const select = screen.getByTestId("strategy-select") as HTMLSelectElement; - select.value = "latency-based-routing"; - select.dispatchEvent(new Event("change", { bubbles: true })); + await user.selectOptions(screen.getByTestId("strategy-select"), "latency-based-routing"); expect(onChange).toHaveBeenCalledWith( expect.objectContaining({ selectedStrategy: "latency-based-routing" }) diff --git a/ui/litellm-dashboard/src/components/router_settings/RoutingStrategySelector.test.tsx b/ui/litellm-dashboard/src/components/router_settings/RoutingStrategySelector.test.tsx index 85b1dc21acf..01f839681f1 100644 --- a/ui/litellm-dashboard/src/components/router_settings/RoutingStrategySelector.test.tsx +++ b/ui/litellm-dashboard/src/components/router_settings/RoutingStrategySelector.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, vi } from "vitest"; import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import RoutingStrategySelector from "./RoutingStrategySelector"; // Ant Design's Select is complex to drive in JSDOM; swap it for a plain @@ -77,21 +78,16 @@ describe("RoutingStrategySelector", () => { it("should not render a description for a strategy that has none", () => { render(); - // "least-busy" has no entry in routingStrategyDescriptions - const select = screen.getByTestId("strategy-select"); - const leastBusyOption = Array.from(select.querySelectorAll("option")).find( - (o) => o.value === "least-busy" - ); - expect(leastBusyOption).toBeInTheDocument(); + // "least-busy" has no entry in routingStrategyDescriptions — it still renders without crashing + expect(screen.getByText("least-busy")).toBeInTheDocument(); }); - it("should call onStrategyChange with the selected strategy value", () => { + it("should call onStrategyChange with the selected strategy value", async () => { const onStrategyChange = vi.fn(); + const user = userEvent.setup(); render(); - const select = screen.getByTestId("strategy-select") as HTMLSelectElement; - select.value = "latency-based-routing"; - select.dispatchEvent(new Event("change", { bubbles: true })); + await user.selectOptions(screen.getByTestId("strategy-select"), "latency-based-routing"); expect(onStrategyChange).toHaveBeenCalledWith("latency-based-routing"); });