fix(#3544): make model selector test resilient to VLLM outages - #3545
fix(#3544): make model selector test resilient to VLLM outages#3545fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
Replace the hardcoded aria snapshot assertion in the model selector
test with a dynamic check that queries /api/lightspeed/v1/models to
discover which models are actually registered. The VLLM provider
endpoint is external and can intermittently return HTTP 503,
preventing the redhataillama model from being registered while the
4 OpenAI models remain available.
The test now fetches the models API to get the current list of LLM
models, asserts at least 4 models are available (the OpenAI
baseline), builds the expected aria snapshot dynamically from the
API response, and uses page.getByRole("menu") instead of
page.locator("body") for a more targeted assertion.
Closes #3544
|
Hi @fullsend-ai-coder[bot]. Thanks for your PR. I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
|
🤖 Finished Review · ✅ Success · Started 10:12 AM UTC · Completed 10:25 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.28 |
ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| .sort(); | ||
|
|
||
| // At least the 4 OpenAI models must always be registered. | ||
| expect(availableModels.length).toBeGreaterThanOrEqual(4); |
There was a problem hiding this comment.
[low] edge-case
The sort order of availableModels uses JavaScript's default lexicographic .sort(), assuming the dropdown UI sorts identically. The original hardcoded list was already in this order, so the assumption holds for current model names. If model names with different casing or special characters are introduced, the snapshot assertion could fail — but this would surface as a clear test failure, not a silent correctness issue.
| "/api/lightspeed/v1/models", | ||
| ); | ||
| expect(modelsResponse.ok()).toBe(true); | ||
| const modelsData = await modelsResponse.json(); |
There was a problem hiding this comment.
[low] error-handling
The API response body is cast via as { type: string; id: string }[] without runtime validation. If the API returns an unexpected shape, a TypeError would surface rather than a descriptive assertion failure. Acceptable in test code.
| // At least the 4 OpenAI models must always be registered. | ||
| expect(availableModels.length).toBeGreaterThanOrEqual(4); | ||
|
|
||
| await dropdown.click(); |
There was a problem hiding this comment.
[low] test-adequacy
selectChatModel(page, "gpt-4o-mini") remains hardcoded, assuming this model is always available. The earlier toBeGreaterThanOrEqual(4) assertion would catch OpenAI provider outages first, and this model is from the stable baseline.
fullsend-ai-review
Bot
added
the
ready-for-merge
|
/ok-to-test |
|
/test ? |
|
/test e2e-ocp-helm-nightly-pr-scoped |
|
/test e2e-ocp-helm-nightly |
❌ Failed Nightly E2E TestsPlatform: ocp 4.20 | RHDH Version: 1.10 | Duration: 41m 15s |
|
@fullsend-ai-coder[bot]: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
github-actions
Bot
added
release-branch-patch
2 participants
Replace the hardcoded aria snapshot assertion in the model selector test with a dynamic check that queries /api/lightspeed/v1/models to discover which models are actually registered. The VLLM provider endpoint is external and can intermittently return HTTP 503, preventing the redhataillama model from being registered while the 4 OpenAI models remain available.
The test now fetches the models API to get the current list of LLM models, asserts at least 4 models are available (the OpenAI baseline), builds the expected aria snapshot dynamically from the API response, and uses page.getByRole("menu") instead of page.locator("body") for a more targeted assertion.
Closes #3544
Post-script verification
agent/3544-dynamic-model-selector)e5b55e10cdf003048a1e38977687ed76a47e2026..HEAD)