[Observability] [RAC] Add basic functional tests for observability alerts #109876
Conversation
|
@elasticmachine merge upstream |
…ibana into 108360-add-alerts-table-tests
|
This is ready for review now. Kerry350@bb9f05c removed the filter check (as we don't have the default filter now), and the archive data has been amended to account for |
Kerry350
added
Feature:Observability RAC
Feature:RAC
|
Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui) |
Kerry350
added
the
release_note:skip
|
|
||
| describe('Alerts table', () => { | ||
| it('Renders the table', async () => { | ||
| await testSubjects.existOrFail('events-viewer-panel'); |
There was a problem hiding this comment.
Would it be a good idea to keep a file of subject selector constants?
There was a problem hiding this comment.
We should create a custom Observability Alerting provider moving forward. Here's an example one: https://github.com/elastic/kibana/blob/master/x-pack/test/functional/services/logs_ui/log_stream.ts, these services let us create functions to perform little selector shortcuts / common interactions etc. I haven't added it for this first addition, but as we'll be adding a lot more functional tests it can come soon. It would be somewhat related to: #110627
There was a problem hiding this comment.
Yeah, I sneakily turned the "set up suites with different permission combinations" task we talked about into actually creating a testing framework service. 😇 I like how the ml team set up a substructure for their page objects/services within their provider.
|
|
||
| it('Renders the correct number of cells', async () => { | ||
| // NOTE: This isn't ideal, but EuiDataGrid doesn't really have the concept of "rows" | ||
| const cells = await testSubjects.findAll('dataGridRowCell'); |
There was a problem hiding this comment.
Does EUI provide some kind of POM/Facades for their components?
Else we might consider in the future creating something like that for ourself. Using selectors that another team specifies could easily lead to our tests breaking and having some layer in between would make it easier to fix.
There was a problem hiding this comment.
I don't think so.
I agree using selectors from other teams is generally not a good idea. However, in this case (and I'm not sure if this is clear at first) the testSubjects service hooks into our data-test-subj attributes (as opposed to random classes / IDs etc), this is a well established pattern in Kibana. Generally speaking EUI don't randomly change the top level data-test-subj attributes on their components. I did look at the time to see if the cells could take a custom data-test-subj as a prop, but it didn't seem to be the case (passing this as a prop to EUI components is very often an option). I might have missed the option somewhere, though.
There was a problem hiding this comment.
I see, thanks for the info!
There was a problem hiding this comment.
The other benefit is that if EUI breaks our tests with a PR, they get to fix them :D
| const pageObjects = getPageObjects(['common']); | ||
| const testSubjects = getService('testSubjects'); | ||
|
|
||
| before(async () => { |
There was a problem hiding this comment.
Is this like beforeEach or beforeAll?
I spoke with Dario yesterday and he mentioned that within APM they have a setup that loads one dataset for all of their functional tests (rather than per suite), is that something we want to copy?
The downside is that the tests risk becoming more coupled but the gain is speed due to less data loading.
There was a problem hiding this comment.
before could be considered the same as beforeAll, it'll run once for the describe block.
is that something we want to copy?
It's a good question. APM's setup is fairly custom (if this is regarding the tests at test/apm_api_integration) where they have their own config, helpers etc. So we could potentially do something similar, but it wouldn't really follow the expected format of tests at test/functional/apps 🤔
There was a problem hiding this comment.
Might be better to stick to conventions then and see how the speed of the test framework may be improved instead.
|
@Kerry350 I was looking at the |
It would make sense for the security / privilege centric tests. I can push some data changes. |
|
I've regenerated the archive data. There's now added variability across consumer / producer and also workflow status. Still a small enough dataset to be able to decompress and inspect if needed though (useful when actually writing tests). I'll merge once green as the only things changed are values to match the new data. |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @Kerry350 |
…erts (elastic#109876) * Add basic functional tests for observability alerts
|
@Kerry350 Do you mention somewhere how we run these tests? |
|
@mgiota You can use the commands here: #111297 (comment), they're just the standard functional test commands (e.g. https://www.elastic.co/guide/en/kibana/current/development-tests.html#development-functional-tests) but with a specific config. |
Summary
This implements #108360. The intention is to fail hard and fail fast if the Observability > Alerts page isn't working as expected.
It adds a new archive which contains some alerts data for some logs, metrics, and APM rules. It also enables
xpack.observability.unsafe.alertingExperience.enabled=truein the xpack config, same asxpack.observability.unsafe.cases.enabled=true.With archives and scaffolding in place it will be much easier to add functional tests on top of this.
Notes
These tests are focussed purely on the Observability > Alerts UI aspect.
With regards to writing of data:
It does look, however, like we should expand the Observability feature controls tests to cover alerts. There are tests for cases already: https://github.com/elastic/kibana/blob/master/x-pack/test/functional/apps/observability/feature_controls/observability_security.ts