[APM] Optimize API test order #88654

dgieselaar · 2021-01-19T10:22:30Z

Conditionally run API tests (depending on which configuration is active), and run them in the most efficient order to minimize time spent loading/unloading archives.

For the trial tests, total time goes from 8m to 2m. ~~Might yield even better returns for basic tests (as there are more).~~ Basic suite goes down to 2m (from 13m).

Obviously somewhat gross but might be worth it.

Here's the general outline:

Expose a runner service (needs a better name) to register tests (using runner.when())
runner.when() accepts a title (for the suite), a condition or more conditions, and the body of the suite (ie, it() and describe())
When all tests have been registered, call runner.run() which will select tests that should run and group tests that need the same archives
For each test group, load and unload the archives once

~~I also need to change the way decorate_snapshot_ui gets the file from the test, as Mocha incorrectly assumes its in the same file as the root suite.~~ Done, using the stack trace now.

spalger

Very excited to see this exploration, I've tried to do something similar many times but get caught up in the details and fear of defining a new interface around Mocha (and the inevitable cries about custom interfaces) but I hope this exploration will lead to some info we can apply to a solution provided by the FTR in the future.

PS: I wouldn't complain if you split the FTR changes out into a separate PR, but they look good to me.

smith

This is great. Works as advertised.

Looks like the jest failures are related to your changes so ok to merge once we're green.

Thanks for doing this.

smith · 2021-01-20T21:57:19Z

x-pack/test/apm_api_integration/tests/traces/top_traces.ts

-import archives_metadata from '../../../common/fixtures/es_archiver/archives_metadata';
-import { FtrProviderContext } from '../../../../common/ftr_provider_context';
+import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata';
+import { FtrProviderContext } from '../../common/ftr_provider_context';

 export default function ApiTest({ getService }: FtrProviderContext) {


nit: Some of these are ApiTest, some are apiTest, and some are somethingApiTest. I prefer lowercase, but it doesn't actually make a difference. Looks like you mostly are preserving what is previously there.

sorenlouv

This is great!

dgieselaar · 2021-01-21T11:25:03Z

@spalger I think that's a good idea, created a separate PR for FTR changes: #88950

dgieselaar · 2021-01-22T09:41:15Z

@elasticmachine merge upstream

…trix-testing

dgieselaar · 2021-01-24T13:33:53Z

@spalger I had to make some changes unfortunately to make it work:

Instead of using a service I'm now using a module. Looks like services are never initialized when running scripts/ensure_all_tests_in_ci_group.js, so the tests using registry.when() never get registered
I initially changed decorate_snapshot_ui to use the stacktrace from expectSnapshot() to get the file for a test. But that means I can only find tests that are not skipped, which means that decorate_snapshot_ui will report false positives for obsolete snapshots. What I'm doing now is using eachTest() for the root suite to iterate over all the tests, and I'm setting test.file when registering a suite in registry.when().

Probably good if you have another look at the changes?

dgieselaar · 2021-01-26T07:11:19Z

@elasticmachine merge upstream

kibanamachine · 2021-01-26T08:56:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 42d4758

Metrics [docs]

✅ unchanged

History

💚 Build #100566 succeeded caf797b
💛 Build #100551 was flaky 5e16d65
💔 Build #100533 failed c54743a
💔 Build #100501 failed 739d3db
💔 Build #100498 failed 8275837

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

spalger

Code looks good, double checked that all tests are running and I'm seeing everything where it should be, but it looks like we might be executing a lot more tests than expected.

Based on the numbers at https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/101001/testReport/ and compared to the test results from the master build of f5e869189b0bbf242637a2fb8f06c1e5e74633b7 (which was merged into this PR most recently) There are now 68 new tests in "APM API integration tests (basic)", and 158 new tests in "X-Pack APM API integration tests (trial)". If these numbers look right to you then LGTM!

dgieselaar · 2021-01-26T19:10:00Z

@spalger I think that's correct, we're registering all tests now but skipping those that are not applicable. The new (skipped) tests in basic are the trial tests, and vice versa.

spalger · 2021-01-26T20:16:28Z

Ah, that makes sense. Would be ideal to skip registering them all together but I get the approach.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

…y-tests * 'master' of github.com:elastic/kibana: (276 commits) [Telemetry] Settings Collector: redact sensitive reported values (elastic#88675) [CI] Combines Jest test jobs (elastic#85850) [Upgrade Assistant] Migrate server to new es-js client (elastic#89207) Migrate maps_legacy, maps_oss, region_map, and tile_map plugions to TS projects (elastic#89351) [Vega Docs] Add experimental flag on the vega maps title (elastic#89402) Increase the time needed to locate the save viz toast (elastic#89301) [Enterprise Search] Add links to doc links service (elastic#89260) Fixed regex bug in Safari (elastic#89399) [Lens] Fix indexpattern checks for missing references (elastic#88840) [Lens] Clean up usage collector (elastic#89109) update apm index pattern (elastic#89395) [APM] Upgrade ES client (elastic#86594) Enable v2 so migrations, disable in FTR tests (elastic#89297) [Search Sessions] Make search session indicator UI opt-in, refactor per-app capabilities (elastic#88699) Cleanup OSS code from visualizations wizard (elastic#89092) [APM] Optimize API test order (elastic#88654) Rename conversion function, extract to module scope and add tests. (elastic#89018) [core.logging] Add ops logs to the KP logging system (elastic#88070) chore(NA): improve ts build refs performance on kbn bootstrap (elastic#89333) skip flaky suite (elastic#89379) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx # x-pack/test/accessibility/config.ts

…ana into task-manager/shift-on-trend * 'task-manager/shift-on-trend' of github.com:gmmorris/kibana: (74 commits) [Metrics UI] Fix Host Overview boxes in Host Detail page (elastic#89299) [Telemetry] Settings Collector: redact sensitive reported values (elastic#88675) [CI] Combines Jest test jobs (elastic#85850) [Upgrade Assistant] Migrate server to new es-js client (elastic#89207) Migrate maps_legacy, maps_oss, region_map, and tile_map plugions to TS projects (elastic#89351) [Vega Docs] Add experimental flag on the vega maps title (elastic#89402) Increase the time needed to locate the save viz toast (elastic#89301) [Enterprise Search] Add links to doc links service (elastic#89260) Fixed regex bug in Safari (elastic#89399) [Lens] Fix indexpattern checks for missing references (elastic#88840) [Lens] Clean up usage collector (elastic#89109) update apm index pattern (elastic#89395) [APM] Upgrade ES client (elastic#86594) Enable v2 so migrations, disable in FTR tests (elastic#89297) [Search Sessions] Make search session indicator UI opt-in, refactor per-app capabilities (elastic#88699) Cleanup OSS code from visualizations wizard (elastic#89092) [APM] Optimize API test order (elastic#88654) Rename conversion function, extract to module scope and add tests. (elastic#89018) [core.logging] Add ops logs to the KP logging system (elastic#88070) chore(NA): improve ts build refs performance on kbn bootstrap (elastic#89333) ...

[APM] POC: Matrix API testing

86a5d9b

dgieselaar changed the title ~~[APM] POC: Matrix API testing~~ [APM] POC: Optimize API test order Jan 19, 2021

dgieselaar added 4 commits January 20, 2021 13:19

Merge branch 'master' of github.com:elastic/kibana into matrix-testing

22864c4

Migrate basic tests

5a9f209

Add tag for CI

c58d668

Merge branch 'master' of github.com:elastic/kibana into matrix-testing

9767517

dgieselaar marked this pull request as ready for review January 20, 2021 20:20

dgieselaar requested a review from a team as a code owner January 20, 2021 20:20

dgieselaar requested a review from a team January 20, 2021 20:20

dgieselaar changed the title ~~[APM] POC: Optimize API test order~~ [APM] Optimize API test order Jan 20, 2021

spalger approved these changes Jan 20, 2021

View reviewed changes

smith approved these changes Jan 20, 2021

View reviewed changes

sorenlouv approved these changes Jan 20, 2021

View reviewed changes

dgieselaar added 3 commits January 21, 2021 18:30

Merge branch 'master' of github.com:elastic/kibana into matrix-testing

f1be529

Update snapshot titles

761e59e

Merge branch 'master' of github.com:elastic/kibana into matrix-testing

8275837

dgieselaar added release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Jan 22, 2021

kibanamachine and others added 8 commits January 22, 2021 04:41

Merge branch 'master' into matrix-testing

739d3db

Use module instead of service to register tests

280c6d4

Merge branch 'matrix-testing' of github.com:dgieselaar/kibana into ma…

c54743a

…trix-testing

Reset registry after/before runs

6c0f6f5

Merge branch 'master' of github.com:elastic/kibana into matrix-testing

e051629

Use test.file and suite.eachTest for snapshot invalidation

ce156c8

Set file of suite in registry

5e16d65

Remove unused snapshots when updating with all

caf797b

dgieselaar requested a review from spalger January 24, 2021 13:33

Merge branch 'master' into matrix-testing

42d4758

spalger approved these changes Jan 26, 2021

View reviewed changes

dgieselaar merged commit 053628c into elastic:master Jan 27, 2021

dgieselaar deleted the matrix-testing branch January 27, 2021 08:54

dgieselaar mentioned this pull request Jan 27, 2021

[7.x] [APM] Optimize API test order (#88654) #89398

Merged

dgieselaar added a commit to dgieselaar/kibana that referenced this pull request Jan 27, 2021

[APM] Optimize API test order (elastic#88654)

7008422

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[APM] Optimize API test order #88654

[APM] Optimize API test order #88654

dgieselaar commented Jan 19, 2021 •

edited

Loading

spalger left a comment

smith left a comment

smith Jan 20, 2021

sorenlouv left a comment

dgieselaar commented Jan 21, 2021

dgieselaar commented Jan 22, 2021

dgieselaar commented Jan 24, 2021 •

edited

Loading

dgieselaar commented Jan 26, 2021

kibanamachine commented Jan 26, 2021

spalger left a comment

dgieselaar commented Jan 26, 2021

spalger commented Jan 26, 2021

[APM] Optimize API test order #88654

[APM] Optimize API test order #88654

Conversation

dgieselaar commented Jan 19, 2021 • edited Loading

spalger left a comment

Choose a reason for hiding this comment

smith left a comment

Choose a reason for hiding this comment

smith Jan 20, 2021

Choose a reason for hiding this comment

sorenlouv left a comment

Choose a reason for hiding this comment

dgieselaar commented Jan 21, 2021

dgieselaar commented Jan 22, 2021

dgieselaar commented Jan 24, 2021 • edited Loading

dgieselaar commented Jan 26, 2021

kibanamachine commented Jan 26, 2021

💚 Build Succeeded

Metrics [docs]

History

spalger left a comment

Choose a reason for hiding this comment

dgieselaar commented Jan 26, 2021

spalger commented Jan 26, 2021

dgieselaar commented Jan 19, 2021 •

edited

Loading

dgieselaar commented Jan 24, 2021 •

edited

Loading