[Reporting] use data-test-subj on toast to check for failure #25482
Conversation
tsullivan
added
bug
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
| screenshot: '[data-shared-item]', | ||
| renderComplete: '[data-shared-item]', | ||
| itemsCountAttribute: 'data-shared-items-count', | ||
| timefilterFromAttribute: 'data-shared-timefilter-from', | ||
| timefilterToAttribute: 'data-shared-timefilter-to', | ||
| toastHeader: '[data-test-subj="euiToastHeader"]', |
There was a problem hiding this comment.
Small nitpick: seems odd to mix selectors with dom-attributes in a selectors object. Not relevant to your PR but had me scratching my head for a second since some values had brackets and some didn't.
There was a problem hiding this comment.
I agree, it seems a little weird that it was originally done this way.
| @@ -59,9 +59,16 @@ export function screenshotsObservableFactory(server) { | |||
| await browser.waitForSelector(`${layout.selectors.renderComplete},[${layout.selectors.itemsCountAttribute}]`); | |||
| }; | |||
|
|
|||
There was a problem hiding this comment.
The name of this function confused me for a bit as I didn't expect it to throw (reading it without context, that is). Maybe something like waitForToastError? I guess something to query the developer that this function will throw when called.
There was a problem hiding this comment.
I had it as-is because we can't reliably differentiate "error" toast messages from any other kind of toast. So if "error" is not going to refer to what we're looking for on the page. This wouldn't be do-able if we don't also grab the message from the toast. The user can review what was there and decide if configuration needs to be fixed, or if was just a flash message and they can re-try again.
How about checkForToastMessage?
There was a problem hiding this comment.
Sounds good to me, and agreed
💔 Build Failed |
|
interesting build failure: |
💔 Build Failed |
💚 Build Succeeded |
…#25482) * [Reporting] use data-test-subj on toast to check for failure * use header subj selector * use toast header text * helper function rename - checkForToastMessage
Depends on: elastic/eui#1302
Resolves: #25418
Summary
This fixes a regression bug in Reporting's detection of Error and Warning toast notifications.
Screenshot
To test:
Checklist
[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] Unit or functional tests were updated or added to match the most common scenarios[ ] This was checked for keyboard-only and screenreader accessibility