fix(testing): eliminate Playwright agents modal test flake (#3333)#3370
Conversation
|
Thanks @omorros. Excellent root cause analysis — the hidden legacy table creating selector ambiguity is a subtle one. The fix is clean: removes dead code and targets by specific IDs. The |
|
@omorros - thank you for your contribution! Can you please resolve the conflict before a review? |
|
Should be solved now! Let me know if there's anything else needed ;) @marekdano |
marekdano
left a comment
There was a problem hiding this comment.
Summary
This PR resolves flaky Playwright tests for the A2A Agents modal by removing a legacy hidden table and using more precise HTMX selectors.
Changes Analysis
- mcpgateway/templates/admin.html (-165 lines)
What Changed:
- Removed entire legacy agents table that was hidden with style="display: none;"
- This table was a duplicate of the HTMX-loaded active table
Why This Fixes the Flake:
- Eliminates selector ambiguity - Playwright could accidentally target the hidden table
- Removes race conditions between two similar DOM structures
- Reduces technical debt
Assessment: ✅ Excellent cleanup - removes dead code that was causing test instability
- tests/playwright/pages/agents_page.py (+3 lines, -1 line)
What Changed:
1 # Before:
2 def agents_table_body(self) -> Locator:
3 return self.agents_table.locator("tbody")
4
5 # After:
6 def agents_table_body(self) -> Locator:
7 return self.page.locator("#agents-table-body")
Why This Fixes the Flake:
- Uses direct ID selector instead of chaining through parent
- More precise targeting of the HTMX-loaded table body
- Added explicit wait for #agents-table-body to be attached before proceeding
Assessment: ✅ Proper fix - follows best practices for HTMX content waiting
Test Coverage
The test file test_agents_extended.py shows comprehensive coverage:
- ✅ 60+ test cases for agent modals (view/edit/test)
- ✅ Proper helper functions with API response interception
- ✅ Explicit waits for modal visibility
- ✅ Tests for all OAuth grant types and auth methods
Recommendations
✅ Approve with Confidence
1. Root cause properly addressed: Removes selector ambiguity
2. No breaking changes: Only removes hidden/unused HTML
3. Follows HTMX best practices: Direct ID selectors + explicit waits
4. Well-tested: Comprehensive test coverage already exists
LGTM 🚀
… to eliminate Playwright modal test flake Signed-off-by: Oriol Morros Vilaseca <OM368@student.aru.ac.uk>
b139068 to
79b2bff
Compare
crivetimihai
left a comment
There was a problem hiding this comment.
Rebased onto main and resolved one conflict in tests/playwright/pages/agents_page.py (chose self.page.locator("#agents-table") to match the consistent pattern used by all other page objects).
Review findings:
- Root cause is correct: the hidden legacy
<table>in the agents panel caused Playwright selector ambiguity when HTMX hadn't finished loading — this is confirmed dead HTML with no JS references, no feature flags, and no code paths to unhide it. - Fix is sound: removing the 159-line dead table eliminates the ambiguity, and the new
wait_for_selector("#agents-table-body", state="attached")ensures the HTMX partial has loaded before tests interact with rows. Usingattachedovervisibleis correct since an empty<tbody>has zero height. - Consistency: all three selector changes (
agents_table,agents_table_body,wait_for_agents_panel_loaded) now match the pattern used across all other page objects (tools, servers, gateways, prompts, resources). - No security or performance concerns — dead code removal + test infrastructure fix only.
- Manual Playwright verification passed: View/Edit/Test modals, search, clear, tab navigation, HTMX table reload all work correctly.
- Automated test: the originally flaky
test_view_modal_different_agents_show_different_datapassed 5/5 consecutive runs.
… to eliminate Playwright modal test flake (#3370) Signed-off-by: Oriol Morros Vilaseca <OM368@student.aru.ac.uk>
… to eliminate Playwright modal test flake (#3370) Signed-off-by: Oriol Morros Vilaseca <OM368@student.aru.ac.uk> Signed-off-by: Yosief Eyob <yosiefogbazion@gmail.com>
… to eliminate Playwright modal test flake (IBM#3370) Signed-off-by: Oriol Morros Vilaseca <OM368@student.aru.ac.uk> Signed-off-by: KRISHNAN, SANTHANA <sk8069@exo.att.com>
🔗 Related Issue
Closes #3333
📝 Summary
Fixes intermittent timeout in
test_view_modal_different_agents_show_different_datacaused by a selectorrace between the HTMX-loaded agents table and a hidden legacy table in
admin.html. Removes the deadlegacy table (159 lines), targets the HTMX table by its specific
#agents-table/#agents-table-bodyIDs, and adds a wait for the HTMX partial to be attached before interacting with agent rows.
🏷️ Type of Change
🧪 Verification
make lintmake testmake coverageTarget test ran 5/5 times with no flakes against a live gateway.
✅ Checklist
make black isort pre-commit)📓 Notes (optional)
Root cause:
agents_tableusedself.agents_panel.locator("table")which matched both the HTMX tableand a hidden legacy table (
style="display: none"). When HTMX hadn't finished loading, Playwright resolvedagainst the hidden table first, finding View buttons that existed but were not actionable.
Changes:
<!-- Old table for reference (hidden) -->block fromadmin.html(lines 6916-7073) — deadcode causing selector ambiguity
agents_table/agents_table_bodyselectors to target#agents-tableand#agents-table-bodydirectly
wait_for_selector("#agents-table-body", state="attached")inwait_for_agents_panel_loaded()toensure the HTMX partial has loaded before tests proceed (using
attachedinstead ofvisiblebecause anempty tbody has zero height)