Skip to content

[Observability] [RAC] Add basic functional tests for observability alerts #109876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kerry350 merged 7 commits into from
Sep 1, 2021
Merged

[Observability] [RAC] Add basic functional tests for observability alerts #109876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4af2c08
Add basic functional tests for observability alerts
Kerry350 Aug 24, 2021
9ab2fbf
Merge branch 'master' into 108360-add-alerts-table-tests
kibanamachine Aug 24, 2021
ce21a13
Merge remote-tracking branch 'upstream/master' into 108360-add-alerts…
Kerry350 Aug 31, 2021
bb9f05c
Remove filter check test and amend archive data to match new field va…
Kerry350 Aug 31, 2021
02f965c
Merge branch '108360-add-alerts-table-tests' of github.com:Kerry350/k…
Kerry350 Aug 31, 2021
ff106fd
Merge remote-tracking branch 'upstream/master' into 108360-add-alerts…
Kerry350 Sep 1, 2021
3cabd63
Regenerate archive data for better varability
Kerry350 Sep 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions x-pack/test/functional/apps/observability/alerts/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import expect from '@kbn/expect';
import querystring from 'querystring';
import { FtrProviderContext } from '../../../ftr_provider_context';

// Based on the x-pack/test/functional/es_archives/observability/alerts archive.
const DATE_WITH_DATA = {
rangeFrom: '2021-08-31T13:36:22.109Z',
rangeTo: '2021-09-01T13:36:22.109Z',
};

export default ({ getPageObjects, getService }: FtrProviderContext) => {
const esArchiver = getService('esArchiver');

describe('Observability alerts', function () {
this.tags('includeFirefox');

const pageObjects = getPageObjects(['common']);
const testSubjects = getService('testSubjects');

before(async () => {
Copy link
Contributor

@miltonhultgren miltonhultgren Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@Kerry350 Kerry350 Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤔

Copy link
Contributor

@miltonhultgren miltonhultgren Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to stick to conventions then and see how the speed of the test framework may be improved instead.

await esArchiver.load('x-pack/test/functional/es_archives/observability/alerts');
await pageObjects.common.navigateToUrlWithBrowserHistory(
'observability',
'/alerts',
`?${querystring.stringify(DATE_WITH_DATA)}`
);
});

after(async () => {
await esArchiver.unload('x-pack/test/functional/es_archives/observability/alerts');
});

describe('Alerts table', () => {
it('Renders the table', async () => {
await testSubjects.existOrFail('events-viewer-panel');
Copy link
Contributor

@miltonhultgren miltonhultgren Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be a good idea to keep a file of subject selector constants?

Copy link
Contributor Author

@Kerry350 Kerry350 Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@weltenwort weltenwort Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@miltonhultgren miltonhultgren Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

});

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');
Copy link
Contributor

@miltonhultgren miltonhultgren Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@Kerry350 Kerry350 Aug 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@miltonhultgren miltonhultgren Sep 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for the info!

Copy link
Member

@simianhacker simianhacker Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other benefit is that if EUI breaks our tests with a PR, they get to fix them :D

expect(cells.length).to.be(54);
});
});
});
};
1 change: 1 addition & 0 deletions x-pack/test/functional/apps/observability/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ export default function ({ loadTestFile }: FtrProviderContext) {
describe('Observability specs', function () {
this.tags('ciGroup6');
loadTestFile(require.resolve('./feature_controls'));
loadTestFile(require.resolve('./alerts'));
});
}
4 changes: 4 additions & 0 deletions x-pack/test/functional/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export default async function ({ readConfigFile }) {
'--timelion.ui.enabled=true',
'--savedObjects.maxImportPayloadBytes=10485760', // for OSS test management/_import_objects
'--xpack.observability.unsafe.cases.enabled=true',
'--xpack.observability.unsafe.alertingExperience.enabled=true', // NOTE: Can be removed once enabled by default
],
},
uiSettings: {
Expand Down Expand Up @@ -207,6 +208,9 @@ export default async function ({ readConfigFile }) {
securitySolution: {
pathname: '/app/security',
},
observability: {
pathname: '/app/observability',
},
},

// choose where screenshots should be saved
Expand Down
Binary file added x-pack/test/functional/es_archives/observability/alerts/data.json.gz
View file
Open in desktop
Binary file not shown.
Loading