Skip to content

Conversation

@surajrdy-aws
Copy link

Problem

Our tests are currently experiencing flakiness due to our usage of try-catch blocks which allow errors to pass through our tests. One of our more egregious examples of this is our quickActions.test.ts file which is unable to find the proper CSS element of our quickAction command overlay, but still allows our test to pass as this error is non-critical due to our try-catch block. This leaves an inherent problem in our tests making them flaky while still containing UI errors. Additionally, many of our helper functions return Booleans, and this logic of booleans is used in some instances like our chat.test.ts.

Solution

We will remove the majority of try-catch blocks that are used in our helpers to have more stricter errors in our tests. Our tests should fail if an error occurs while finding a UI element. Additionally, we update our chat.test.ts so that it can properly stop using the Boolean returned from the chat helper function.

We have updated:

  • generalUtils
  • cleanupUtils
  • chat.test.ts
  • quickActionsHelper
  • switchModelHelper

  • 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.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@surajrdy-aws surajrdy-aws requested a review from a team as a code owner August 13, 2025 19:37
@github-actions
Copy link

  • This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
    • Note: beta or "experiment" features that have active users should announce fixes in the changelog.
    • If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

Copy link

@laura-codess laura-codess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks like the only logic changed is just removing the try catch blocks

}
for (const button of closeButtons) {
await button.click()
await sleep(500)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: look into this sleep.

Copy link
Contributor

@Hweinstock Hweinstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for making this change! I think the error flow will be much more straightforward.

* @throws Error if timeout occurs before response is detected
*/
export async function waitForChatResponse(webview: WebviewView, timeout = 8000): Promise<boolean> {
export async function waitForChatResponse(webview: WebviewView, timeout = 15000): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, is there a reason we need to increase the default here?

@bywang56 bywang56 merged commit 9db3a91 into aws:feature/ui-e2e-tests Aug 15, 2025
30 of 38 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.

4 participants