feat(amazonq): Improve the button clicking logic, replaced the MCP test suite (#7881)

laura-codess · web-flow · commit 2baf460c8b8c · 2025-08-14T13:59:09.000-07:00
## Changes I researched how we can make a very robust button clicking logic using Selenium best practices. I added this new general function into generalUtils.ts and replaced the MCP test suite with this logic so it no longer flakes. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
diff --git a/packages/amazonq/test/e2e_new/amazonq/helpers/mcpHelper.ts b/packages/amazonq/test/e2e_new/amazonq/helpers/mcpHelper.ts
@@ -3,31 +3,21 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 import { WebviewView, By, WebElement } from 'vscode-extension-tester'
-import { waitForElement } from '../utils/generalUtils'
+import { clickButton, waitForElement } from '../utils/generalUtils'
+import { dismissOverlayIfPresent } from '../utils/cleanupUtils'
 
 /**
  * Clicks the tools to get to the MCP server overlay
  * @param webviewView The WebviewView instance
  * @returns Promise<boolean> True if tools button was found and clicked, false otherwise
  */
 export async function clickToolsButton(webviewView: WebviewView): Promise<void> {
-    try {
-        const navWrapper = await waitForElement(webviewView, By.css('.mynah-nav-tabs-wrapper.mynah-ui-clickable-item'))
-        const buttonsWrapper = await navWrapper.findElement(By.css('.mynah-nav-tabs-bar-buttons-wrapper'))
-        const buttons = await buttonsWrapper.findElements(
-            By.css('.mynah-button.mynah-button-secondary.fill-state-always.mynah-ui-clickable-item')
-        )
-        for (const button of buttons) {
-            const icon = await button.findElement(By.css('i.mynah-ui-icon.mynah-ui-icon-tools'))
-            if (icon) {
-                await button.click()
-                await webviewView.getDriver().actions().move({ x: 0, y: 0 }).perform()
-            }
-        }
-        console.log('Tools button not found')
-    } catch (e) {
-        console.error('Error clicking tools button:', e)
-    }
+    await clickButton(
+        webviewView,
+        '[data-testid="tab-bar-buttons-wrapper"]',
+        '[data-testid="tab-bar-button"] .mynah-ui-icon-tools',
+        'tools button'
+    )
 }
 
 /**
@@ -36,15 +26,7 @@ export async function clickToolsButton(webviewView: WebviewView): Promise<void>
  * @returns Promise<boolean> True if add button was found and clicked, false otherwise
  */
 export async function clickMCPAddButton(webviewView: WebviewView): Promise<void> {
-    try {
-        const sheetWrapper = await waitForElement(webviewView, By.id('mynah-sheet-wrapper'))
-        const header = await sheetWrapper.findElement(By.css('.mynah-sheet-header'))
-        const actionsContainer = await header.findElement(By.css('.mynah-sheet-header-actions-container'))
-        const addButton = await actionsContainer.findElement(By.css('button:has(i.mynah-ui-icon-plus)'))
-        await addButton.click()
-    } catch (e) {
-        console.error('Error clicking the MCP add button:', e)
-    }
+    await clickButton(webviewView, '.mynah-sheet-header-actions-container', 'i.mynah-ui-icon-plus', 'MCP add button')
 }
 
 /**
@@ -225,17 +207,12 @@ export async function configureMCPServer(webviewView: WebviewView, config: MCPSe
 }
 
 export async function saveMCPServerConfiguration(webviewView: WebviewView): Promise<void> {
-    try {
-        const sheetWrapper = await waitForElement(webviewView, By.id('mynah-sheet-wrapper'))
-        const body = await sheetWrapper.findElement(By.css('.mynah-sheet-body'))
-        const filterActions = await body.findElement(By.css('.mynah-detailed-list-filter-actions-wrapper'))
-        const saveButton = await filterActions.findElement(
-            By.css('.mynah-button.fill-state-always.status-primary.mynah-ui-clickable-item')
-        )
-        await saveButton.click()
-    } catch (e) {
-        console.error('Error saving the MCP server configuration:', e)
-    }
+    await clickButton(
+        webviewView,
+        '[data-testid="chat-item-action-button"][action-id="save-mcp"]',
+        'span.mynah-button-label',
+        'save button'
+    )
 }
 
 export async function cancelMCPServerConfiguration(webviewView: WebviewView): Promise<void> {
@@ -259,13 +236,21 @@ export async function cancelMCPServerConfiguration(webviewView: WebviewView): Pr
  */
 export async function clickMCPRefreshButton(webviewView: WebviewView): Promise<void> {
     try {
-        const sheetWrapper = await waitForElement(webviewView, By.id('mynah-sheet-wrapper'))
-        const header = await sheetWrapper.findElement(By.css('.mynah-sheet-header'))
-        const actionsContainer = await header.findElement(By.css('.mynah-sheet-header-actions-container'))
-        const refreshButton = await actionsContainer.findElement(By.css('button:has(i.mynah-ui-icon-refresh)'))
-        await refreshButton.click()
+        // First dismiss any overlay that might be present
+        await dismissOverlayIfPresent(webviewView)
+
+        await clickButton(
+            webviewView,
+            '.mynah-sheet-header-actions-container',
+            'i.mynah-ui-icon-refresh',
+            'MCP refresh button'
+        )
+
+        // Dismiss any overlay that might appear after clicking
+        await dismissOverlayIfPresent(webviewView)
     } catch (e) {
         console.error('Error clicking the MCP refresh button:', e)
+        throw e
     }
 }
 
@@ -276,11 +261,10 @@ export async function clickMCPRefreshButton(webviewView: WebviewView): Promise<v
  */
 export async function clickMCPCloseButton(webviewView: WebviewView): Promise<void> {
     try {
-        const sheetWrapper = await waitForElement(webviewView, By.id('mynah-sheet-wrapper'))
-        const header = await sheetWrapper.findElement(By.css('.mynah-sheet-header'))
-        const cancelButton = await header.findElement(By.css('button:has(i.mynah-ui-icon-cancel)'))
-        await webviewView.getDriver().executeScript('arguments[0].click()', cancelButton)
+        await dismissOverlayIfPresent(webviewView)
+        await clickButton(webviewView, '.mynah-sheet-header', 'i.mynah-ui-icon-cancel', 'MCP close button')
     } catch (e) {
         console.error('Error closing the MCP overlay:', e)
+        throw e
     }
 }
diff --git a/packages/amazonq/test/e2e_new/amazonq/tests/mcp.test.ts b/packages/amazonq/test/e2e_new/amazonq/tests/mcp.test.ts
@@ -9,9 +9,9 @@ import {
     clickMCPAddButton,
     clickMCPCloseButton,
     clickMCPRefreshButton,
-    clickToolsButton,
     configureMCPServer,
     saveMCPServerConfiguration,
+    clickToolsButton,
 } from '../helpers/mcpHelper'
 import { closeAllTabs } from '../utils/cleanupUtils'
 
diff --git a/packages/amazonq/test/e2e_new/amazonq/utils/generalUtils.ts b/packages/amazonq/test/e2e_new/amazonq/utils/generalUtils.ts
@@ -39,6 +39,54 @@ export async function waitForElements(webview: WebviewView, locator: By, timeout
     return await webview.findWebElements(locator)
 }
 
+/**
+ * Robust button clicking function that locates a button by its wrapper and content, then clicks it
+ * @param webviewView The WebviewView instance
+ * @param buttonWrapperSelector CSS selector for the button's wrapper element
+ * @param buttonContentSelector CSS selector for the content inside the button (icon, text, etc.)
+ * @param buttonName Descriptive name for the button (used in error messages)
+ * @param timeout Timeout in milliseconds (defaults to 5000)
+ * @returns Promise<void>
+ */
+export async function clickButton(
+    webviewView: WebviewView,
+    buttonWrapperSelector: string,
+    buttonContentSelector: string,
+    buttonName: string = 'button',
+    timeout: number = 5000
+): Promise<void> {
+    try {
+        const buttonWrapper = await webviewView
+            .getDriver()
+            .wait(until.elementLocated(By.css(buttonWrapperSelector)), timeout, `${buttonName} wrapper not found`)
+
+        await webviewView
+            .getDriver()
+            .wait(until.elementIsVisible(buttonWrapper), timeout, `${buttonName} wrapper not visible`)
+
+        const buttonContent = await webviewView
+            .getDriver()
+            .wait(until.elementLocated(By.css(buttonContentSelector)), timeout, `${buttonName} content not found`)
+
+        const button = await buttonContent.findElement(By.xpath('./..'))
+        await webviewView.getDriver().wait(until.elementIsEnabled(button), timeout, `${buttonName} not clickable`)
+        await button.click()
+        await webviewView.getDriver().sleep(300)
+    } catch (e) {
+        console.error(`Failed to click ${buttonName}:`, {
+            error: e,
+            timestamp: new Date().toISOString(),
+        })
+        try {
+            const screenshot = await webviewView.getDriver().takeScreenshot()
+            console.log(`Screenshot taken at time of ${buttonName} failure`, screenshot)
+        } catch (screenshotError) {
+            console.error('Failed to take error screenshot:', screenshotError)
+        }
+        throw new Error(`Failed to click ${buttonName}: ${e}`)
+    }
+}
+
 /**
  * Presses a single key globally
  * @param driver The WebDriver instance
