-
Notifications
You must be signed in to change notification settings - Fork 192
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
chore: attempt to make tab closing less flaky #6029
Conversation
.$(Selectors.workspaceTab(selectorOptions)) | ||
.$(Selectors.CloseWorkspaceTab) | ||
.click(); | ||
await browser.clickVisible( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clickVisible does some more waiting/checks
const id = await currentActiveTab.getAttribute('id'); | ||
await closeTab(browser, { id }, autoConfirmTabClose); | ||
} | ||
await browser.waitUntil(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waitUntil is the better way to poll/repeat than a tight loop. Takes into account proper timeouts.
const currentActiveTab = await browser.$( | ||
Selectors.workspaceTab({ active: true }) | ||
); | ||
await currentActiveTab.click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think this click was doing anything because the tab's already active.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when I wrote it, the close button would only show up when the tab header was focused, but not when it's just active, it might've been a bug that we fixed at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also changed this significantly a few days ago which is likely why it started flaking again and also I could have just missed this.
const currentActiveTab = await browser.$( | ||
Selectors.workspaceTab({ active: true }) | ||
); | ||
await currentActiveTab.click(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when I wrote it, the close button would only show up when the tab header was focused, but not when it's just active, it might've been a bug that we fixed at some point
No description provided.