Remove flaky selectors: .ng-scope, .ng-binding and .ng-isolate.scope #19688
Conversation
|
These were not updated but they are failing: test/functional/page_objects/visualize_page.js: const selector = Was not sure how to remove and keep the equivalent functionality. |
💔 Build Failed |
|
jenkins test this |
| @@ -198,7 +198,7 @@ export function DiscoverPageProvider({ getService, getPageObjects }) { | |||
|
|
|||
| getDocHeader() { | |||
| return getRemote() | |||
| .findByCssSelector('thead.ng-isolate-scope > tr:nth-child(1)') | |||
| .findByCssSelector('thead > tr:nth-child(1)') | |||
There was a problem hiding this comment.
👍 I only see one thead on the page, so there's no chance that this could be accidentally selecting a different thead that happens to not have .ng-isolate-scope applied.
| @@ -225,7 +225,7 @@ export function HeaderPageProvider({ getService, getPageObjects }) { | |||
|
|
|||
| async getToastMessage(findTimeout = defaultFindTimeout) { | |||
| const toastMessage = | |||
| await find.displayedByCssSelector('kbn-truncated.toast-message.ng-isolate-scope', findTimeout); | |||
| await find.displayedByCssSelector('kbn-truncated.toast-message', findTimeout); | |||
There was a problem hiding this comment.
👍 Likewise, the toaster notifications are a singleton, so there's no chance we could be accidentally selecting a different element here.
| @@ -39,7 +39,7 @@ export function MonitoringPageProvider({ getService }) { | |||
|
|
|||
| getToasterContents() { | |||
| return getRemote() | |||
| .findByCssSelector('div.toaster-container.ng-isolate-scope') | |||
| .findByCssSelector('div.toaster-container') | |||
| @@ -128,7 +128,7 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
| } | |||
|
|
|||
| async getChartTypeCount() { | |||
There was a problem hiding this comment.
I don't believe this function is ever called, and the wizard-vis-type class doesn't actually exist in our CSS any more. Can we delete this function entirely in this PR?
There was a problem hiding this comment.
Even though it is not used, I will leave it up to the visualization team to remove it, if they don't intend to use it.
| @@ -394,7 +394,7 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
| // clickBucket(bucketType) 'X-Axis', 'Split Area', 'Split Chart' | |||
| async clickBucket(bucketName) { | |||
| const chartTypes = await retry.try( | |||
| async () => await find.allByCssSelector('li.list-group-item.list-group-menu-item.ng-binding.ng-scope')); | |||
| async () => await find.allByCssSelector('li.list-group-item.list-group-menu-item')); | |||
There was a problem hiding this comment.
👍 This is extremely brittle, but I don't see any elements with this selector which could be inadvertently selected.
This method clicks an item in this list:
These items already have data-test-subj attributes on them, so we should refactor this logic to leverage them in a later PR.
There was a problem hiding this comment.
Agree, a later PR to address adding data-test-subj sounds best. This PR is just a quick fix to get things running on cloud.
| @@ -442,7 +442,7 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
| const aggItem = await find.byCssSelector(`[data-test-subj="${agg}"]`); | |||
| await aggItem.click(); | |||
| const fieldSelect = await find | |||
| .byCssSelector(`#visAggEditorParams${index} > [agg-param="agg.type.params[0]"] > div > div > div.ui-select-match.ng-scope > span`); | |||
| .byCssSelector(`#visAggEditorParams${index} > [agg-param="agg.type.params[0]"] > div > div > div.ui-select-match > span`); | |||
There was a problem hiding this comment.
👍 This selector's high specificity and dependence upon ui-select class names makes it unlikely it could accidentally select the wrong element.
| @@ -510,7 +510,7 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
| async orderBy(fieldValue) { | |||
| await find.clickByCssSelector( | |||
| 'select.form-control.ng-pristine.ng-valid.ng-untouched.ng-valid-required[ng-model="agg.params.orderBy"]' | |||
| + `option.ng-binding.ng-scope:contains("${fieldValue}")`); | |||
| + `option:contains("${fieldValue}")`); | |||
| @@ -817,12 +817,12 @@ export function VisualizePageProvider({ getService, getPageObjects }) { | |||
| } | |||
|
|
|||
| async getMarkdownData() { | |||
| const markdown = await retry.try(async () => find.byCssSelector('visualize.ng-isolate-scope')); | |||
| const markdown = await retry.try(async () => find.byCssSelector('visualize')); | |||
There was a problem hiding this comment.
👍 There's only one instance of <visualize> on the page at a time, so this should be fine too.
| return await markdown.getVisibleText(); | ||
| } | ||
|
|
||
| async clickColumns() { | ||
| await find.clickByCssSelector('div.schemaEditors.ng-scope > div > div > button:nth-child(2)'); | ||
| await find.clickByCssSelector('div.schemaEditors > div > div > button:nth-child(2)'); |
💚 Build Succeeded |
This is a fix for #19679 to remove the use of .ng-scope, .ng-binding in selenium selectors that can make the tests flaky / brittle. We should move towards using data-test-subj. This is just a temporary fix to get the tests running on cloud.