Skip to content

test(e2e): cover add-MCP-server flow via discovery → custom form#29070

Merged
ryan-crabbe-berri merged 4 commits into
litellm_internal_stagingfrom
litellm_e2e_mcp_add_server
May 28, 2026
Merged

test(e2e): cover add-MCP-server flow via discovery → custom form#29070
ryan-crabbe-berri merged 4 commits into
litellm_internal_stagingfrom
litellm_e2e_mcp_add_server

Conversation

@ryan-crabbe-berri

@ryan-crabbe-berri ryan-crabbe-berri commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds an e2e test for the previously-uncovered manual-QA item "Add MCP server". The test opens the discovery modal, drops into the custom-server form, fills a unique server name + Streamable HTTP transport + a placeholder URL + None auth, submits, and verifies both the success toast and the new row in the table.

Test plan

  • e2e_tests/tests/mcp/mcpServers.spec.ts::Add a custom MCP server via the discovery → custom form

Refs LIT-3093

The "Add MCP server" manual-QA step was uncovered. This adds a test
that opens the discovery modal, jumps into the custom-server form,
fills name + Streamable HTTP transport + a placeholder URL + None
auth, submits, and verifies both the success toast and the new row.
@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps

greptile-apps Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a single new Playwright E2E test covering the previously-untested "Add MCP server" flow: opening the discovery modal, switching to the custom-server form, selecting Streamable HTTP transport and None auth, and verifying the success toast plus the new row in the table.

  • The test is purely additive — no production code is changed.
  • Locator anchors are well-chosen (.ant-collapse-item for the auth section, { exact: true } on the "None" option) and directly address the fragility concerns raised in earlier review rounds; inline comments explain each non-obvious choice.
  • The two-modal transition (setDiscoveryVisible(false) + setModalVisible(true) batched in one render) is handled correctly by the lazy formModal locator combined with the 5 s toBeVisible timeout.

Confidence Score: 5/5

Safe to merge — the change is a single additive E2E test file with no production code touched.

Only one new test file is introduced, no existing code is modified, and the test exercises a UI flow that was previously untested. The locator strategy is sound and the previous review concerns have been explicitly addressed in code comments.

No files require special attention.

Important Files Changed

Filename Overview
ui/litellm-dashboard/e2e_tests/tests/mcp/mcpServers.spec.ts New E2E test for the MCP server "Add" happy path — discovery modal → custom form → Streamable HTTP + None auth → success toast + table row assertion. Well-commented locator choices, unique timestamped name, and no teardown needed due to fresh-DB runner.

Reviews (2): Last reviewed commit: "docs(e2e): note MCP coverage scope and l..." | Re-trigger Greptile

Comment thread ui/litellm-dashboard/e2e_tests/tests/mcp/mcpServers.spec.ts Outdated
Comment thread ui/litellm-dashboard/e2e_tests/tests/mcp/mcpServers.spec.ts Outdated
- Anchor the auth-type Select via its enclosing Collapse panel
  ("Authentication") instead of the placeholder text. The Form.Item has
  no label prop, so the previous `hasText: /auth type/i` filter was
  matching via "Select auth type" placeholder copy — fragile.
- Document the intentional lack of teardown, matching the pattern used
  in addModel.spec.ts: the e2e runner discards the DB per invocation.

Addresses Greptile P2s on PR #29070.
Scope the post-create row lookup to `table tbody` so the form modal's
`server_name` input — which still holds the timestamped value during
its close animation — can't satisfy the assertion before the server
actually lands in the list.
This spec only smoke-tests the happy-path Streamable HTTP + None auth
flow. Add a top-of-file comment pointing at E2E_COVERAGE.md so future
contributors can see what's still uncovered (other transports, all
auth types, edit/delete, BYOK, tool list/call, access groups).
@ryan-crabbe-berri

Copy link
Copy Markdown
Collaborator Author

@greptileai re review

@ryan-crabbe-berri ryan-crabbe-berri merged commit bc31c57 into litellm_internal_staging May 28, 2026
115 of 118 checks passed
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