[Security_Solution][Resolver] Resolver loading and error state #75600

michaelolo24 · 2020-08-20T20:12:29Z

Summary

Basic tests added for the different states of the resolver. Loading, Error, and Success.

General Changes

Updated the extended jest toYieldEqualTo to require a successful view state to remain successful for the remaining iterations of the event loop to be successful. I can update this to require success for x number of iterations if we decide to go that route.
Updated one clickthrough test that was failing based on the above

Loading Tests
This test utilizes pause functionality within the mock dataAccessLayer, which will load the resolver with configured requests in a paused state. This allows us to check that the loading state occurs and is maintained while awaiting resolution of those requests.

Error Test
This test utilizes and emptiable dataAccessLayer that allows configuration of which requests return an empty set of data. Based on the logic in resolver_tree_fetcher.ts an error will occur if no entities are returned, but if an empty tree is returned the resolver will show a blank window (this scenario should never occur since the entityID request would need to be successful for the resolverTree request to even run).

Success Test
Simple test that just makes sure all nodes loaded in the graph successfully.

Checklist

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

x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx

x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx

elasticmachine · 2020-08-24T21:16:18Z

Pinging @elastic/endpoint-app-team (Feature:Resolver)

elasticmachine · 2020-08-24T21:16:19Z

Pinging @elastic/endpoint-data-visibility-team (Team:Endpoint Data Visibility)

x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts

oatkiller

Added comments with a few thoughts.

...ugins/security_solution/public/resolver/data_access_layer/mocks/no_resolver_data_returned.ts

x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts

x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx

x-pack/plugins/security_solution/public/resolver/view/resolver_loading_state.test.tsx

oatkiller · 2020-08-25T18:33:49Z

x-pack/plugins/security_solution/public/resolver/view/resolver_loading_state.test.tsx

+      // Trigger a loading state by requesting data based on a new DocumentID.
+      // There really is no way to do this in the view besides changing the url, so triggering the action instead
+
+      simulator.requestNewTree(newTreeId);


You could change the database document ID prop using setProp from enzyme. That way you don't have to have public simulator methods that rely on the internals of Resolver (I'm considering the props to the Resolver component as public.)

see below comment

oatkiller · 2020-08-25T18:35:54Z

x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx

+   * Find the graph container by the database document ID.
+   * This better allows us to test when new data is loaded into an existing resolver
+   */
+  public processResolverGraph(databaseDocumentID: string): ReactWrapper {


Another possible approach. Create a mock data access layer that returns different data based on the databaseDocumentID. Load resolver, then change the databaseDocumentID props. New data will be loaded. You will be able to verify that based on the data in the panel and the graph. maybe?

I like this idea, but the more I thought about it, I realized I would be testing functionality that we don't currently have in the UI, as far as I can tell, so figured it would be best to leave this till we figure out what that implementation may look like. 🤷‍♂️

michaelolo24 · 2020-08-26T17:35:03Z

x-pack/plugins/security_solution/public/resolver/view/resolver_loading_state.test.tsx

+    });
+  });
+
+  describe("When the resolver tree request doesn't return any data", () => {


This should really never happen since the resolverTree request only takes place after successfully getting an entityID. Have this test here though so we're at least checking the behavior in case something wonky happens.

This could happen if different indices are used for entities than what are used for resolverTree. I think that might even be the case today.

bkimmel · 2020-08-27T13:11:22Z

...rity_solution/public/resolver/data_access_layer/mocks/emptiable_no_ancestors_two_children.ts

+}
+
+/**
+ * A simple mock dataAccessLayer that allows you to configure which requests return an empty set of data.


ℹ️ good idea. These empty kinds seem to give us a lot of trouble, We should use this a lot in tests.

bkimmel · 2020-08-27T13:26:02Z

x-pack/plugins/security_solution/public/resolver/view/resolver_loading_state.test.tsx

+
+    it('should display a loading state', async () => {
+      // Trigger a loading state by requesting data based on a new DocumentID.
+      // There really is no way to do this in the view besides changing the url, so triggering the action instead


❔ Where is the action mentioned in this comment being triggered?

errant comment from an earlier implementation. Gonna remove, thanks!

bkimmel · 2020-08-27T13:38:26Z

...urity_solution/public/resolver/data_access_layer/mocks/pausable_no_ancestors_two_children.ts

+}
+
+/**
+ * A simple mock dataAccessLayer that allows you to manually pause and resume a request.


❔ These emptiable and pausable DALs are really cool. Just wondering if there is a way we could make those concepts composable somehow, like if you wanted something to delay an empty request you could do something like Emptiable(Pausable(mockTreeWithTwoAncestors(), {relatedEvents: true}), {relatedEvent: true})

oatkiller

left a few suggestions

oatkiller · 2020-08-27T17:43:01Z

x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/emptify_mock.ts

+}
+
+/**
+ * A simple mock dataAccessLayer that allows you to manually pause and resume a request.


This comment is out of date.

x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/emptify_mock.ts

oatkiller · 2020-08-27T17:46:42Z

x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/emptify_mock.ts

+      async entities(): Promise<ResolverEntityIndex> {
+        return dataShouldBeEmpty?.includes('entities')
+          ? Promise.resolve([])
+          : // @ts-ignore - ignore the argument requirement for dataAccessLayer


what error is happening here exactly?

Thanks, updated to just pass args through.

I think you forgot to remove the @ts-ignore after fixing the issue

Also you can switch to @ts-expect-error now

x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/pausify_mock.ts

oatkiller · 2020-08-27T18:05:21Z

x-pack/plugins/security_solution/public/resolver/view/resolver_loading_state.test.tsx

+    });
+  });
+
+  describe("When the resolver tree request doesn't return any data", () => {


This could happen if different indices are used for entities than what are used for resolverTree. I think that might even be the case today.

x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/emptify_mock.ts

oatkiller

Nice

kibanamachine · 2020-08-27T20:32:52Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 26dfa65

Build metrics

distributable file count

id	value	diff	baseline
total	53128	+2	53126

History

💚 Build #70996 succeeded 389f912
💚 Build #70986 succeeded 62aead1
💔 Build #70967 failed a314610ac8345ecb38045e8604955c165425acef
💚 Build #70610 succeeded d3306a03da1a26911237939ecc82fc9a5919872f
💔 Build #70588 failed 3b0a1a9eca96d21505038006a0eefe9cf898531d

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

…ic#75600)

…s-for-710 * 'master' of github.com:elastic/kibana: (43 commits) [APM] Chart units don't update when toggling the chart legends (elastic#74931) [ILM] Add support for frozen phase in UI (elastic#75968) Hides advanced json for count metric (elastic#74636) add client-side feature usage API (elastic#75486) [Maps] add drilldown support map embeddable (elastic#75598) [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106) [Resolver] model `location.search` in redux (elastic#76140) [APM] Prevent imports of public in server code (elastic#75979) fix eslint issue skip flaky suite (elastic#76223) [APM] Transaction duration anomaly alerting integration (elastic#75719) [Transforms] Avoid using "Are you sure" (elastic#75932) [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145) [plugin-helpers] improve 3rd party KP plugin support (elastic#75019) [docs/getting-started] link to yarn v1 specifically (elastic#76169) [Security_Solution][Resolver] Resolver loading and error state (elastic#75600) Fixes App Search documentation links (elastic#76133) Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079) [Resolver] Fix useSelector usage (elastic#76129) [Enterprise Search] Migrate util and components from ent-search (elastic#76051) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts # x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts

kibanamachine · 2020-08-31T20:48:45Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

…75600) (#76192) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

michaelolo24 force-pushed the resolver-loading-states branch 2 times, most recently from 31e59e6 to 2bf8258 Compare August 24, 2020 18:43

michaelolo24 commented Aug 24, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx Outdated Show resolved Hide resolved

michaelolo24 force-pushed the resolver-loading-states branch 2 times, most recently from 0894e34 to d45a17b Compare August 24, 2020 19:47

michaelolo24 changed the title ~~WIP: need better control flow~~ [Security_Solution][Resolver] Resolver loading and error state Aug 24, 2020

michaelolo24 commented Aug 24, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx Show resolved Hide resolved

michaelolo24 force-pushed the resolver-loading-states branch 2 times, most recently from 6d22c28 to f04ace5 Compare August 24, 2020 21:09

michaelolo24 added Feature:Resolver Security Solution Resolver feature release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Data Visibility Team managing the endpoint resolver v7.10.0 v8.0.0 labels Aug 24, 2020

michaelolo24 marked this pull request as ready for review August 24, 2020 21:16

michaelolo24 requested review from a team as code owners August 24, 2020 21:16

michaelolo24 commented Aug 24, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts Outdated Show resolved Hide resolved

michaelolo24 force-pushed the resolver-loading-states branch from e9507d4 to b850601 Compare August 25, 2020 15:59

oatkiller reviewed Aug 25, 2020

View reviewed changes

michaelolo24 commented Aug 26, 2020

View reviewed changes

bkimmel reviewed Aug 27, 2020

View reviewed changes

bkimmel approved these changes Aug 27, 2020

View reviewed changes

michaelolo24 added 3 commits August 27, 2020 12:00

WIP: need better control flow

6c2519c

WIP: added error data access layer

544de76

cleaned up tests

2da7c52

michaelolo24 added 6 commits August 27, 2020 12:00

fix comment

b841c2a

improved pattern

0f03731

order fix

062454f

types fix

1a69f70

make pause and empty composable

0485a73

updated comment & fix pausify

62aead1

michaelolo24 force-pushed the resolver-loading-states branch from e41ea6b to 62aead1 Compare August 27, 2020 18:02

oatkiller reviewed Aug 27, 2020

View reviewed changes

pr feedback cleanup

f75a0f9

oatkiller reviewed Aug 27, 2020

View reviewed changes

x-pack/plugins/security_solution/public/resolver/data_access_layer/mocks/emptify_mock.ts Outdated Show resolved Hide resolved

michaelolo24 added 2 commits August 27, 2020 12:39

remove ts-ignore

26dfa65

make emptify arg required

389f912

oatkiller approved these changes Aug 27, 2020

View reviewed changes

michaelolo24 merged commit 280fb8b into elastic:master Aug 27, 2020

michaelolo24 deleted the resolver-loading-states branch August 27, 2020 20:42

michaelolo24 mentioned this pull request Aug 27, 2020

[7.x] [Security_Solution][Resolver] Resolver loading and error state (#75600) #76192

Merged

michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Aug 27, 2020

[Security_Solution][Resolver] Resolver loading and error state (elast…

9b3215a

…ic#75600)

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 31, 2020

michaelolo24 added a commit that referenced this pull request Sep 1, 2020

[7.x] [Security_Solution][Resolver] Resolver loading and error state (#…

20c6efe

…75600) (#76192) Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Sep 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security_Solution][Resolver] Resolver loading and error state #75600

[Security_Solution][Resolver] Resolver loading and error state #75600

michaelolo24 commented Aug 20, 2020 •

edited

Loading

elasticmachine commented Aug 24, 2020

elasticmachine commented Aug 24, 2020

oatkiller left a comment

oatkiller Aug 25, 2020

michaelolo24 Aug 26, 2020

oatkiller Aug 25, 2020

michaelolo24 Aug 26, 2020

michaelolo24 Aug 26, 2020

oatkiller Aug 27, 2020

bkimmel Aug 27, 2020

bkimmel Aug 27, 2020

michaelolo24 Aug 27, 2020

bkimmel Aug 27, 2020

oatkiller left a comment

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

michaelolo24 Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller Aug 27, 2020

oatkiller left a comment

kibanamachine commented Aug 27, 2020

kibanamachine commented Aug 31, 2020

[Security_Solution][Resolver] Resolver loading and error state #75600

[Security_Solution][Resolver] Resolver loading and error state #75600

Conversation

michaelolo24 commented Aug 20, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Aug 24, 2020

elasticmachine commented Aug 24, 2020

oatkiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oatkiller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oatkiller left a comment

Choose a reason for hiding this comment

kibanamachine commented Aug 27, 2020

💚 Build Succeeded

Build metrics

distributable file count

History

kibanamachine commented Aug 31, 2020

michaelolo24 commented Aug 20, 2020 •

edited

Loading