[ftr/obs/alerts] refactor to avoid stale-element errors

[spalger]

Fixes #140248

Stale element exceptions are caused by fetching a reference to a browser element and then interacting with it some time later, after that element no longer exists in the DOM.

This can happen with code like this:

// fetch a reference to the btn
const btn = await testSubjects.find('...')
// click the button via reference, if the element was removed from the DOM between these two calls this will throw a `StaleElementReferenceError`
await btn.click();

To avoid this the testSubjects service exposes several methods that allow operating on elements in common ways without capturing a reference to the element itself, and if a StaleElementReferenceError occurs internally the relevant operation can be retried automatically. To make functional testing code resilient to these StaleElementReferenceErrors we should instead write the above code like this:

// let the testSubjects service make sure the click is delivered and retry on `StaleElementReferenceError`
await testSubjects.click('...')

Additionally, there is a retryOnStale() helper function available. This function simply wraps an async function and calls it up to 10 times, returning it's result as long as it doesn't throw a StaleElementReferenceError. This can be useful for more complex functions like the ones in the ObservabilityAlertsCommonProvider, where element references are used in abundance and refactoring to use compound functions from the testSubjects service isn't easy.

In order to address #140248 I moved the retryOnStale helper function to a service so it could be easily accessed by the ObservabilityAlertsCommonProvider. Then, I either converted the call to use the compound helper functions from testSubjects or wrapped an existing method with the helper so that they will automatically retry if a StaleElementReferenceError is thrown like in #140248.

[elasticmachine]

Pinging @elastic/kibana-operations (Team:Operations)

[jasonrhodes]

This LGTM from the observability changes side. @spenceralger can you point us to the best place to go to better understand retryOnStale.wrap and the testSubjects.* methods as safer alternatives to something like return await (await getQuerySubmitButton()).click(); ?

[spalger]

@jasonrhodes Honestly, there isn't a great place to learn about the testSubjects service, and the retryOnStale.wrap() function is something I wrote recently and is until this point undocumented. I'll add a comprehensive functional testing guide to our documentation matrix so that we won't forget to get to it eventually, but at this point the best docs we have are at https://www.elastic.co/guide/en/kibana/current/development-tests.html#development-functional-tests and then of course there's always the source code and the types.

(BTW, I'm @spalger)

[jasonrhodes]

@spalger ok sounds good, we'll look there. And I know your GH tag, no idea why my fingers just decided to write spencer 🤷

[spalger]

200 successful builds in https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1198, running another 200 in https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1199

[maryam-saeidi]

@spalger Interesting finding, I like to help you with documenting best practices for functional tests and potentially check the observability functional tests to make sure they match the best practices. Let me know how I can help.

[cnasikas]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: a65c34c

Metrics [docs]

✅ unchanged

History

• 💚 Build #71041 succeeded 63caeb1
• 💚 Build #70988 succeeded d516138
• 💔 Build #70967 failed 26a646b
• 💚 Build #70943 succeeded 8a2a9f6

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[dmlemeshko]

LGTM

[dmlemeshko]

Just a note: unfortunately testSubjects.click() still won't keep it safe of StaleElementExcetion in 100%, though it guarantees the 'new' reference to element is used for actions like click/type/etc. It is important to have a mechanism to properly wait for page loading before the findElement call, so that element won't be updated in DOM when it is least expected. I think we should explain it in the docs.