Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(e2e): stabilize Amazon Q feature development tests #6520

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chengoramazon
Copy link
Contributor

Problem

Amazon Q feature dev e2e test was disabled due to flakiness. Issue: #6513

Solution

  • Modified test prompt to prevent no change required scenarios that were causing deadend.
  • Add getSelectedTab function in qTestingFramework to access newly created tab by quick action.

  • 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.

@chengoramazon
Copy link
Contributor Author

/runIntegrationTests

@chengoramazon
Copy link
Contributor Author

/runIntegrationTests

@chengoramazon
Copy link
Contributor Author

chengoramazon commented Feb 6, 2025

Failed test in E2E stable is unrelated to /dev

"before all" hook in "Amazon Q Code Transformation

@chengoramazon chengoramazon marked this pull request as ready for review February 6, 2025 21:38
@chengoramazon chengoramazon requested a review from a team as a code owner February 6, 2025 21:38
beforeEach(async function () {
this.timeout(maxTestDuration)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anyway we can avoid increasing the timeout? The default timeout is 5 minutes, so increasing it to 10 minutes in this beforeEach increases it up to 5 minutes per test in this block. Combined with the change above, this could add up to 50 minutes to execution time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can revert the timeout increase given the logs showed each test can complete within 5 mins. Please note /dev is a long run which can go beyond, we may revisit this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you confirmed that these e2e tests are reliable without these increased timeouts? From the toolkits side, we are very reluctant to merge tests that cause inconsistencies in our CI due to the confusion and churn it creates.

Copy link
Contributor Author

@chengoramazon chengoramazon Feb 7, 2025

Choose a reason for hiding this comment

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

Confirmed. Tried a couple of runs which the new prompt can finish without timeout increase. The previous flakiness was mainly from a dead-end situation.

@chengoramazon
Copy link
Contributor Author

/runIntegrationTests

Comment on lines +108 to +116
public getSelectedTab() {
const selectedTabId = this.mynahUI.getSelectedTabId()
const selectedTab = this.getTabs().find((tab) => tab.tabID === selectedTabId)

if (!selectedTab) {
assert.fail('Selected tab not found')
}
return selectedTab
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate more on why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See reply above

beforeEach(async function () {
tab = framework.createTab()
Copy link
Contributor

Choose a reason for hiding this comment

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

why are the two tabs here and line 226 different? are they always different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a new tab created when a /dev quick action triggered. The getSelectedTab can help get the current selected tab;

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