[Application Usage] Functional test to validate the full list of appIds in the schema by afharo · Pull Request #88080 · elastic/kibana

afharo · 2021-01-12T19:25:36Z

Summary

This PR adds functional tests to the Application Usage schema definition: it runs Kibana and checks that all the full list of registered apps is detailed in the schema (all appIds should be set as keys in the applicationUsage schema).

Resolves #75329

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

afharo · 2021-01-12T19:29:40Z

x-pack/test/usage_collection/config.ts

I think that this new folder of functional tests can be used as well when implementing #83704

…ds in the schema

afharo · 2021-01-15T10:31:01Z

@elasticmachine merge upstream

…-schema

joshdover

Just be sure the test suite is completely set up for CI correctly, have we tested making pushing a failing version of the test to CI? Sometimes it's easy to miss a detail about adding a new functional suite and I like to use that as a gut-check.

joshdover · 2021-01-15T11:53:11Z

x-pack/test/usage_collection/plugins/application_usage_test/public/plugin.ts

+    try {
+      // When the lists don't match, the entire page load will fail.
+      expect(Object.keys(applicationUsageSchema).sort()).to.eql(appIds);
+    } catch (err) {
+      err.message = `Application Usage's schema is not up-to-date with the actual registered apps. Please update it at src/plugins/kibana_usage_collection/server/collectors/application_usage/schema.ts.\n${err.message}`;
+      throw err;
+    }


Instead of throwing an error on startup, it may be nicer to have this plugin set a property on window.__usageCollectionTest__ or similar and have the test read that property. That way, other tests that we add to this suite can still run even if this test is currently failing.

That makes perfect sense! In fact, I had a thought about it, but couldn't find a way to read the window object in the tests and thought it was not possible. Now that you mentioned, I ran a second search and found the right way to do it. Thanks! I'll push the changes in a bit.

Re the CI, I've checked Jenkins' logs to make sure the test runs. And I tried locally the fail scenario in the test. I'll push a failed scenario anyway to ensure we are setting things right by seeing the CI fail 👍

The CI failed as expected: https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/99129/testReport/ 🎉

Running a last build with the correct schema.

afharo · 2021-01-18T08:41:45Z

@elasticmachine merge upstream

…-schema

pgayvallet

LGTM

pgayvallet · 2021-01-19T11:44:05Z

src/plugins/kibana_usage_collection/server/collectors/application_usage/schema.ts


-// These keys obtained by searching for `/application\w*\.register\(/` and checking the value of the attr `id`.
-// TODO: Find a way to update these keys automatically.
+// There is a test in x-pack/test/usage_collection that validates that the keys in here match all the registered apps


As adding a new app will cause the test to fail, maybe there should be a comment on the test pointing to this file to help developers fix it when they'll add new apps?

@pgayvallet makes sense! I've updated the test to point at the file in the error message.

Here's an example of the output:

0 passing (10.4s) │1 failing │ │1) Application Usage │ keys in the schema match the registered application IDs: │ │ │ Error: Application Usage's schema is not up-to-date with the actual registered apps. Please update it at src/plugins/kibana_usage_collection/server/collectors/application_usage/schema.ts. │ expected [ 'apm', │ 'appSearch', │ 'canvas', │ 'dashboard_mode', │ 'dashboards', │ 'dev_tools', │ 'discover', │ 'enterpriseSearch', │ 'error', │ 'fleet', │ 'graph', │ 'home', │ 'infra', │ 'ingestManager', │ 'kibana', │ 'kibanaOverview', │ 'lens', │ 'logs', │ 'management', │ 'maps', │ 'metrics', │ 'ml', │ 'monitoring', │ 'observability-overview', │ 'securitySolution', │ 'securitySolution:administration', │ 'securitySolution:case', │ 'securitySolution:detections', │ 'securitySolution:hosts', │ 'securitySolution:network', │ 'securitySolution:overview', │ 'securitySolution:timelines', │ 'security_access_agreement', │ 'security_account', │ 'security_capture_url', │ 'security_logged_out', │ 'security_login', │ 'security_logout', │ 'security_overwritten_session', │ 'short_url_redirect', │ 'siem', │ 'space_selector', │ 'status', │ 'test', │ 'timelion', │ 'uptime', │ 'ux', │ 'visualize', │ 'workplaceSearch' ] to sort of equal [ 'apm', │ 'appSearch', │ 'canvas', │ 'dashboard_mode', │ 'dashboards', │ 'dev_tools', │ 'discover', │ 'enterpriseSearch', │ 'error', │ 'fleet', │ 'graph', │ 'home', │ 'infra', │ 'ingestManager', │ 'kibana', │ 'kibanaOverview', │ 'lens', │ 'logs', │ 'management', │ 'maps', │ 'metrics', │ 'ml', │ 'monitoring', │ 'observability-overview', │ 'securitySolution', │ 'securitySolution:administration', │ 'securitySolution:case', │ 'securitySolution:detections', │ 'securitySolution:hosts', │ 'securitySolution:network', │ 'securitySolution:overview', │ 'securitySolution:timelines', │ 'security_access_agreement', │ 'security_account', │ 'security_capture_url', │ 'security_logged_out', │ 'security_login', │ 'security_logout', │ 'security_overwritten_session', │ 'short_url_redirect', │ 'siem', │ 'space_selector', │ 'status', │ 'timelion', │ 'uptime', │ 'ux', │ 'visualize', │ 'workplaceSearch' ] │ + expected - actual │ │ "short_url_redirect" │ "siem" │ "space_selector" │ "status" │ - "test" │ "timelion" │ "uptime" │ "ux" │ "visualize" │ │ at Assertion.assert (/Users/afharo/Developer/elastic/kibana/packages/kbn-expect/expect.js:100:11) │ at Assertion.eql (/Users/afharo/Developer/elastic/kibana/packages/kbn-expect/expect.js:244:8) │ at Context.<anonymous> (test/usage_collection/test_suites/application_usage/index.ts:21:63) │ at runMicrotasks (<anonymous>) │ at processTicksAndRejections (internal/process/task_queues.js:93:5) │ at Object.apply (/Users/afharo/Developer/elastic/kibana/packages/kbn-test/src/functional_test_runner/lib/mocha/wrap_function.js:84:16) │ │

Perfect! thanks.

…sage/functiona-test-appIds-in-schema

kibanamachine · 2021-01-19T14:43:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4406dee

Metrics [docs]

✅ unchanged

History

💚 Build #99353 succeeded c44c628
💚 Build #99180 succeeded 4ccc488
💔 Build #99129 failed 3d4915f
💚 Build #99080 succeeded 1f3ddd8
💚 Build #98621 succeeded 28902a7

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

…f appIds in the schema (#88080) (#88681) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

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

afharo commented Jan 12, 2021

View reviewed changes

afharo added 2 commits January 13, 2021 10:21

[Application Usage] Functional test to validate the full list of appI…

3365808

…ds in the schema

Enable test in the CI

5f41295

afharo force-pushed the application_usage/functiona-test-appIds-in-schema branch from 04f3052 to 5f41295 Compare January 13, 2021 09:32

afharo marked this pull request as ready for review January 13, 2021 13:41

afharo requested a review from a team as a code owner January 13, 2021 13:41

afharo added the Feature:Telemetry label Jan 13, 2021

No need to filter out 'test' appIds

28902a7

Merge branch 'master' into application_usage/functiona-test-appIds-in…

1f3ddd8

…-schema

joshdover reviewed Jan 15, 2021

View reviewed changes

afharo added 3 commits January 15, 2021 15:35

Test the apps in the test suite instead of the plugin

7e0df18

TO BE REVERTED: Extra key in schema to make CI fail

3d4915f

Remove extra key for all green

4ccc488

afharo requested a review from joshdover January 15, 2021 18:39

Merge branch 'master' into application_usage/functiona-test-appIds-in…

c44c628

…-schema

pgayvallet approved these changes Jan 19, 2021

View reviewed changes

afharo added 2 commits January 19, 2021 12:50

Merge branch 'master' of github.com:elastic/kibana into application_u…

280e880

…sage/functiona-test-appIds-in-schema

Error message pointing to the location of the schema to fix

4406dee

afharo merged commit 7c9b114 into elastic:master Jan 19, 2021

afharo deleted the application_usage/functiona-test-appIds-in-schema branch January 19, 2021 14:44

afharo mentioned this pull request Jan 19, 2021

[7.x] [Application Usage] Functional test to validate the full list of appIds in the schema (#88080) #88681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Application Usage] Functional test to validate the full list of appIds in the schema#88080

[Application Usage] Functional test to validate the full list of appIds in the schema#88080
afharo merged 10 commits intoelastic:masterfrom
afharo:application_usage/functiona-test-appIds-in-schema

afharo commented Jan 12, 2021 •

edited

Loading

Uh oh!

afharo Jan 12, 2021

Uh oh!

afharo commented Jan 15, 2021

Uh oh!

joshdover left a comment

Uh oh!

joshdover Jan 15, 2021 •

edited

Loading

Uh oh!

afharo Jan 15, 2021

Uh oh!

afharo Jan 15, 2021

Uh oh!

afharo commented Jan 18, 2021

Uh oh!

pgayvallet left a comment

Uh oh!

pgayvallet Jan 19, 2021

Uh oh!

afharo Jan 19, 2021

Uh oh!

pgayvallet Jan 19, 2021

Uh oh!

kibanamachine commented Jan 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

afharo commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

afharo Jan 12, 2021

Choose a reason for hiding this comment

Uh oh!

afharo commented Jan 15, 2021

Uh oh!

joshdover left a comment

Choose a reason for hiding this comment

Uh oh!

joshdover Jan 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afharo Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

afharo Jan 15, 2021

Choose a reason for hiding this comment

Uh oh!

afharo commented Jan 18, 2021

Uh oh!

pgayvallet left a comment

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

afharo Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

pgayvallet Jan 19, 2021

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jan 19, 2021

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

afharo commented Jan 12, 2021 •

edited

Loading

joshdover Jan 15, 2021 •

edited

Loading