[Logs / Metrics UI] [Alerting] Add functional tests

[Kerry350]

With the work in #67157 changes were made to our logs / metrics feature registration around alerting. There was a bug present, but as we don't have any functional tests it wasn't flagged.

For both logs and metrics alerts we should add some functional tests that assert the following:

• You can load the logs and metrics apps and add an alert via the alerts action menu.
• If you then head to the alerts management page these alerts can be edited.
• New logs and metrics alerts can be created directly via the alerts management page.

These functional tests don't need to be exhaustive in the sense of every comparator option and so on (unit tests can cover this), what we're trying to cover here is the actual top level ability to save and edit alerts.

[elasticmachine]

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

[jasonrhodes]

Big +1 for this, although I'm worried about the trade-offs of how long it will take to write and run these vs how reliable they'll be after all of that ... let's set 1-2 of these up and look at how stable they look like they'll be and go from there.

I also think we need tests for some of the more "interesting" things we're doing with alerts, e.g. "alert if no data".

I wonder if @elastic/kibana-alerting-services could help us think through how to do better integration-level testing that doesn't require Selenium/browsers/real network latency, etc...

[Kerry350]

@jasonrhodes

I'm worried about the trade-offs of how long it will take to write and run these vs how reliable they'll be after all of that

Yeah, this is always a concern with integration tests.

Ideally, I'd like to see these tests cover the things that happen at the alert framework level, rather than at our executor level (where control is sort of placed back in our apps' hands, rather than anything framework related - this isn't 100% true since you still access alert instances from the executor, but more or less it's us controlling things).

This would mean things shout loud and quickly if they break after alerting changes (#67157 being a good example). Similarly, they might have flagged #71828 which is something that was happening at the framework level of validation (although only if that particular comparator was tested for).

I think a good two case scenario is:

• Can an alert be added from our apps
• Can an alert be added from the alerts management page

We don't have to try and cover every single comparator combination etc, but just have some baseline that breaks aggressively when a team changes some code we integrate with.

On the logs side, I think everything else can be covered pretty well with thorough executor unit tests. Although #69261 is needed to expand those as they were written before group by was introduced. Here we can do the whole "every single combination works" tests, cheaply.

I think that would strike a good balance between coverage, and the fact integration tests aren't easy to write or maintain.

[mikecote]

The pattern we've learned on the alerting team is to have less checks within the end to end tests and more within the API integration tests. For example, you can make sure the executor works as designed based on different configurations using the API integration tests and have an end to end test to ensure you can create an alert in the UI and assert on the created alert returned from the /alert/{id} API.

You can find an example of alert executor testing here: https://github.com/elastic/kibana/blob/master/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/alerts_base.ts#L77-L137.

You can find an example of end to end testing here: https://github.com/elastic/kibana/blob/master/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/alerts.ts#L48-L105. There's some changes we wish to do on this test, relying less on the browser API calls to do assertions and more calling the server APIs directly like in the API integration tests.

There's a lot of edge cases that can cause flakiness. Especially with a background task manager and finding the proper hooks to wait for before doing assertions. We can definitely elaborate on more information in regards to testing, hopefully the above is a starting point.

[Kerry350]

@mikecote That’s really useful, thank you!

[jasonrhodes]

@mikecote that's great! Do you have any docs for the alert utils outside of just reading this file: https://github.com/elastic/kibana/blob/master/x-pack/test/alerting_api_integration/common/lib/alert_utils.ts ?

[mikecote]

@jasonrhodes we unfortunately don't have anything. Though, we'd be happy to help whoever wants to leverage it.