-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Reduce flakiness of input control vis test by using testSubjects.click. #21318
Reduce flakiness of input control vis test by using testSubjects.click. #21318
Conversation
…testSubjects service instead of find.
await PageObjects.common.sleep(1000); | ||
} | ||
|
||
async setComboBoxElement(element, value) { | ||
const input = await element.findByTagName('input'); | ||
async setComboBoxElement(comboBox, value) { |
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 had to refactor out common functionality between setComboBox
and setComboBoxElement
into setComboBoxValue
and then remove the dependency upon setComboBoxElement
from setComboBox
so that we could pass down the test subject selector to the other helper methods.
} | ||
|
||
async getComboBoxOptions(comboBoxSelector) { | ||
await testSubjects.click(comboBoxSelector); | ||
const menu = await retry.try( | ||
async () => await testSubjects.find('comboBoxOptionsList')); | ||
const optionsText = await menu.getVisibleText(); | ||
const comboBox = await testSubjects.find(comboBoxSelector); | ||
await this.closeComboBoxOptionsList(comboBox); |
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 this comboBox
might become stale in some situations.
@@ -301,35 +311,32 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
|
|||
async clearComboBox(comboBoxSelector) { | |||
log.debug(`clearComboBox for comboBoxSelector:${comboBoxSelector}`); | |||
const comboBox = await testSubjects.find(comboBoxSelector); |
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.
Same here.
if (!clearButtonExists) { | ||
log.debug('Unable to clear comboBox, comboBoxClearButton does not exist'); | ||
return; | ||
} | ||
|
||
const clearBtn = await comboBox.findByCssSelector('[data-test-subj="comboBoxClearButton"]'); | ||
await clearBtn.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.
Maybe clearBtn
could also become stale?
💚 Build Succeeded |
Retest |
💚 Build Succeeded |
Retest |
💚 Build Succeeded |
Retest |
💚 Build Succeeded |
Retest |
💚 Build Succeeded |
@@ -248,17 +248,28 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
|
|||
async setComboBox(comboBoxSelector, value) { | |||
const comboBox = await testSubjects.find(comboBoxSelector); | |||
await this.setComboBoxElement(comboBox, value); | |||
await this.setComboBoxValue(comboBox, value); | |||
await this.closeComboBoxOptionsList(comboBoxSelector); | |||
await PageObjects.common.sleep(1000); |
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.
wonder if we could get rid of this sleep as part of this code?
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'll try it out.
await this.setComboBoxValue(comboBox, value); | ||
|
||
// Close list. | ||
const isOptionsListOpen = await testSubjects.exists('comboBoxOptionsList'); |
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.
Can closeComboBoxOptionsList
function be used here instead?
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 don't think so, since that method requires you to pass in a data-test-subj selector, and we don't have access to one in this method (we only have access to the combo box element itself).
I created elastic/eui#1054 so we can select the specific options list associated with a particular combo box. Once that's merged we can come back and update these tests, probably in a separate PR. |
💚 Build Succeeded |
@stacey-gammon Ready for another look. |
looks like we may be working towards the same goal. I have a PR out to move the combo box logic into its own service, #21219. The PR also removes the sleeps from the combo box methods. |
Closing in favor of #21219 |
Also simplified code by using testSubjects service instead of find. Fixes #21310.
I'm not sure what exactly was causing the original issue, but it looks like the test called for the combo box value to be set to a new option, which wasn't taking effect but also didn't cause the test to fail. I think that by using the
testSubjects
service instead offind
, we'll be able to ensure elements are available before trying to click on them, and avoid clicking on stale references.