mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(e2e): only the negative fallback assertion needs every replica to agree
litellm-e2e-ui 68 failed the test this PR was meant to stabilise: "fallback never took effect", streak 4 of a required 5, 60s timeout. Requiring a consecutive streak of 200s after the fallback is set was wrong. It asserts that the fallback path succeeds five times running, which is a reliability claim the test never intended to make, and the path is inherently retry-ish because the broken primary is attempted first on every call. One intermittent non-200 resets the streak, so a mostly-working fallback never converges. The two directions are not symmetric: before the write proving NO replica serves it -> needs every replica after the write proving the fallback serves it -> one success is the claim So the control keeps a multi-sample window and the success assertion goes back to polling for a first sighting, on the wider 60s budget rather than the original 30s that expired on litellm-e2e-ui 63. Also drops the two local rebinds Greptile flagged against the repo's no-reassignment convention: the streak counter is gone with the helper it lived in, and the cache-round loop is now a lazy generator consumed by next().
This commit is contained in:
parent
84dfc18f6b
commit
b95801172c
2 changed files with 26 additions and 30 deletions
|
|
@ -102,11 +102,8 @@ class TestCostHeaders:
|
|||
return response
|
||||
return None
|
||||
|
||||
measured: StreamingResponse | None = None
|
||||
for _ in range(CACHE_ATTEMPTS):
|
||||
measured = prime_then_reread()
|
||||
if measured is not None:
|
||||
break
|
||||
rounds = (prime_then_reread() for _ in range(CACHE_ATTEMPTS))
|
||||
measured = next((response for response in rounds if response is not None), None)
|
||||
if measured is None:
|
||||
pytest.fail(
|
||||
f"no cache read landed across {CACHE_ATTEMPTS} prime rounds of "
|
||||
|
|
|
|||
|
|
@ -139,24 +139,18 @@ async function patchRouterSettings(
|
|||
}
|
||||
|
||||
/**
|
||||
* Requires a consecutive streak because a single reply only proves the one replica that
|
||||
* served it has reloaded, not the sibling still answering from the pre-update config.
|
||||
* Spreads its samples across more than one reload cycle: a single reply only proves the one
|
||||
* replica that served it has reloaded, not the sibling still on the pre-update config.
|
||||
*/
|
||||
async function pollUntilSettled(
|
||||
probe: () => Promise<number>,
|
||||
matches: (status: number) => boolean,
|
||||
message: string,
|
||||
): Promise<void> {
|
||||
let streak = 0;
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
streak = matches(await probe()) ? streak + 1 : 0;
|
||||
return streak;
|
||||
},
|
||||
{ timeout: SETTLE_TIMEOUT_MS, intervals: [SETTLE_INTERVAL_MS], message },
|
||||
)
|
||||
.toBeGreaterThanOrEqual(SETTLE_PROBES);
|
||||
async function sampleStatuses(probe: () => Promise<number>): Promise<readonly number[]> {
|
||||
return Array.from({ length: SETTLE_PROBES }).reduce<Promise<readonly number[]>>(
|
||||
async (taken, _unused, index) => {
|
||||
const sofar = await taken;
|
||||
if (index > 0) await new Promise((resolve) => setTimeout(resolve, SETTLE_INTERVAL_MS));
|
||||
return [...sofar, await probe()];
|
||||
},
|
||||
Promise.resolve([]),
|
||||
);
|
||||
}
|
||||
|
||||
test.describe("Router Settings - Loadbalancing", () => {
|
||||
|
|
@ -289,19 +283,24 @@ test.describe("Router Settings - Fallbacks serve the request", () => {
|
|||
})
|
||||
).status();
|
||||
|
||||
// The control: it proves the reply below could only have come from the fallback.
|
||||
await pollUntilSettled(
|
||||
chatStatus,
|
||||
(status) => status >= 400,
|
||||
"broken primary unexpectedly succeeded on its own",
|
||||
);
|
||||
// The control: every replica must reject, or the reply below could have come from one
|
||||
// that was still serving a fallback left behind by an earlier attempt.
|
||||
await expect
|
||||
.poll(async () => (await sampleStatuses(chatStatus)).every((status) => status >= 400), {
|
||||
timeout: SETTLE_TIMEOUT_MS,
|
||||
message: "broken primary unexpectedly succeeded on its own",
|
||||
})
|
||||
.toBe(true);
|
||||
|
||||
await patchRouterSettings(request, {
|
||||
fallbacks: [{ [BROKEN_PRIMARY]: [PRIMARY] }],
|
||||
} as Partial<NonNullable<ConfigYAML["router_settings"]>>);
|
||||
|
||||
// Same call now succeeds, served by the fallback model.
|
||||
await pollUntilSettled(chatStatus, (status) => status === 200, "fallback never took effect");
|
||||
// One success is the whole claim here, so this waits for a first sighting rather than
|
||||
// for every replica: demanding a streak would also assert a fallback hit rate.
|
||||
await expect
|
||||
.poll(chatStatus, { timeout: SETTLE_TIMEOUT_MS, message: "fallback never took effect" })
|
||||
.toBe(200);
|
||||
|
||||
// And the playground renders a reply for a model whose own upstream is down.
|
||||
await openPlayground(page);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue