Skip to content
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

Jump to bottom

[Security Solution][Resolver] Graph Control Tests and Update Simulator Selectors #74680

Merged
merged 9 commits into from
Aug 13, 2020
Merged

[Security Solution][Resolver] Graph Control Tests and Update Simulator Selectors #74680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a0aa654
initialized graph control tests
michaelolo24 Aug 10, 2020
72ad3d4
added zoom tests
michaelolo24 Aug 10, 2020
57eac4b
fix comments
michaelolo24 Aug 10, 2020
9a84f5a
make graph control tests more explicit
michaelolo24 Aug 10, 2020
96be89e
refactor to use domNodes and map
michaelolo24 Aug 11, 2020
e1b9523
slider, not button
michaelolo24 Aug 11, 2020
f3376f8
Add .toYieldObjectEqualTo matcher. resolve graph control test wrapper…
Aug 12, 2020
df4ca51
simplify selector logic
michaelolo24 Aug 12, 2020
505a348
replaced old method references
michaelolo24 Aug 13, 2020
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
65 changes: 65 additions & 0 deletions x-pack/plugins/security_solution/public/resolver/test_utilities/extend_jest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ declare global {
namespace jest {
interface Matchers<R, T> {
toYieldEqualTo(expectedYield: T extends AsyncIterable<infer E> ? E : never): Promise<R>;
toYieldObjectEqualTo(expectedYield: unknown): Promise<R>;
}
}
}
Expand Down Expand Up @@ -57,6 +58,70 @@ expect.extend({
}
}

// Use `pass` as set in the above loop (or initialized to `false`)
// See https://jestjs.io/docs/en/expect#custom-matchers-api and https://jestjs.io/docs/en/expect#thisutils
const message = pass
? () =>
`${this.utils.matcherHint(matcherName, undefined, undefined, options)}\n\n` +
`Expected: not ${this.utils.printExpected(expected)}\n${
this.utils.stringify(expected) !== this.utils.stringify(received[received.length - 1]!)
? `Received: ${this.utils.printReceived(received[received.length - 1])}`
: ''
}`
: () =>
`${this.utils.matcherHint(matcherName, undefined, undefined, options)}\n\nCompared ${
received.length
} yields.\n\n${received
.map(
(next, index) =>
`yield ${index + 1}:\n\n${this.utils.printDiffOrStringify(
expected,
next,
'Expected',
'Received',
this.expand
)}`
)
.join(`\n\n`)}`;

return { message, pass };
},
/**
* A custom matcher that takes an async generator and compares each value it yields to an expected value.
* This uses the same equality logic as `toMatchObject`.
* If any yielded value equals the expected value, the matcher will pass.
* If the generator ends with none of the yielded values matching, it will fail.
*/
async toYieldObjectEqualTo<T>(
this: jest.MatcherContext,
receivedIterable: AsyncIterable<T>,
expected: T
): Promise<{ pass: boolean; message: () => string }> {
// Used in printing out the pass or fail message
const matcherName = 'toSometimesYieldEqualTo';
const options: jest.MatcherHintOptions = {
comment: 'deep equality with any yielded value',
isNot: this.isNot,
promise: this.promise,
};
// The last value received: Used in printing the message
const received: T[] = [];

// Set to true if the test passes.
let pass: boolean = false;

// Async iterate over the iterable
for await (const next of receivedIterable) {
// keep track of all received values. Used in pass and fail messages
received.push(next);
// Use deep equals to compare the value to the expected value
if ((this.equals(next, expected), [this.utils.iterableEquality, this.utils.subsetEquality])) {
// If the value is equal, break
pass = true;
break;
}
}

// Use `pass` as set in the above loop (or initialized to `false`)
// See https://jestjs.io/docs/en/expect#custom-matchers-api and https://jestjs.io/docs/en/expect#thisutils
const message = pass
Expand Down
97 changes: 33 additions & 64 deletions x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import { spyMiddlewareFactory } from '../spy_middleware_factory';
import { resolverMiddlewareFactory } from '../../store/middleware';
import { resolverReducer } from '../../store/reducer';
import { MockResolver } from './mock_resolver';
import { ResolverState, DataAccessLayer, SpyMiddleware } from '../../types';
import { ResolverState, DataAccessLayer, SpyMiddleware, SideEffectSimulator } from '../../types';
import { ResolverAction } from '../../store/actions';
import { sideEffectSimulatorFactory } from '../../view/side_effect_simulator_factory';

/**
* Test a Resolver instance using jest, enzyme, and a mock data layer.
Expand Down Expand Up @@ -43,6 +44,11 @@ export class Simulator {
* This is used by `debugActions`.
*/
private readonly spyMiddleware: SpyMiddleware;
/**
* Simulator which allows you to explicitly simulate resize events and trigger animation frames
*/
private readonly sideEffectSimulator: SideEffectSimulator;

constructor({
dataAccessLayer,
resolverComponentInstanceID,
Expand Down Expand Up @@ -87,11 +93,14 @@ export class Simulator {
// Used for `KibanaContextProvider`
const coreStart: CoreStart = coreMock.createStart();

this.sideEffectSimulator = sideEffectSimulatorFactory();

// Render Resolver via the `MockResolver` component, using `enzyme`.
this.wrapper = mount(
<MockResolver
resolverComponentInstanceID={this.resolverComponentInstanceID}
history={this.history}
sideEffectSimulator={this.sideEffectSimulator}
store={this.store}
coreStart={coreStart}
databaseDocumentID={databaseDocumentID}
Expand Down Expand Up @@ -149,6 +158,18 @@ export class Simulator {
return this.domNodes(processNodeElementSelector(options));
}

/**
* Return an Enzyme ReactWrapper for any child elements of a specific processNodeElement
*
* @param entityID The entity ID of the proocess node to select in
* @param selector The selector for the child element of the process node
*/
public processNodeChildElements(entityID: string, selector: string): ReactWrapper {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bkimmel, thoughts? I wanted to generalize your method a bit for any other buttons or things that might be added to the process node in the future

Copy link
Contributor

Choose a reason for hiding this comment

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

❔ It looks good, generally. Two points:

  1. The parameter isn't really a selector (it only feeds the data-test-subj attrib). So I'd say adjust the comment.
  2. To make that even more clear, you could put a "ByDataTestSubj" at the end of the method name (but that's awful long)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, we can talk about these patterns later today as well. Gonna merge this for now

return this.domNodes(
`${processNodeElementSelector({ entityID })} [data-test-subj="${selector}"]`
);
}

/**
* Return the node element with the given `entityID`.
*/
Expand All @@ -174,21 +195,11 @@ export class Simulator {
}

/**
* Return an Enzyme ReactWrapper that includes the Related Events host button for a given process node
*
* @param entityID The entity ID of the proocess node to select in
* This manually runs the animation frames tied to a configurable timestamp in the future
*/
public processNodeRelatedEventButton(entityID: string): ReactWrapper {
return this.domNodes(
`${processNodeElementSelector({ entityID })} [data-test-subj="resolver:submenu:button"]`
);
}

/**
* The items in the submenu that is opened by expanding a node in the map.
*/
public processNodeSubmenuItems(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:map:node-submenu-item"]');
public runAnimationFramesTimeFromNow(time: number = 0) {
this.sideEffectSimulator.controls.time = time;
this.sideEffectSimulator.controls.provideAnimationFrame();
}

/**
Expand All @@ -202,59 +213,17 @@ export class Simulator {
}

/**
* The element that shows when Resolver is waiting for the graph data.
*/
public graphLoadingElement(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:graph:loading"]');
}

/**
* The element that shows if Resolver couldn't draw the graph.
*/
public graphErrorElement(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:graph:error"]');
}

/**
* The element where nodes get drawn.
*/
public graphElement(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:graph"]');
}

/**
* The titles of the links that select a node in the node list view.
*/
public nodeListNodeLinkText(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:node-list:node-link:title"]');
}

/**
* The icons in the links that select a node in the node list view.
*/
public nodeListNodeLinkIcons(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:node-list:node-link:icon"]');
}

/**
* Link rendered in the breadcrumbs of the node detail view. Takes the user to the node list.
*/
public nodeDetailBreadcrumbNodeListLink(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:node-detail:breadcrumbs:node-list-link"]');
}

/**
* The title element for the node detail view.
* Given a 'data-test-subj' value, it will resolve the react wrapper or undefined if not found
*/
public nodeDetailViewTitle(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:node-detail:title"]');
public async resolve(selector: string): Promise<ReactWrapper | undefined> {
return this.resolveWrapper(() => this.domNodes(`[data-test-subj="${selector}"]`));
}

/**
* The icon element for the node detail title.
* Given a 'data-test-subj' selector, it will return the domNode
*/
public nodeDetailViewTitleIcon(): ReactWrapper {
return this.domNodes('[data-test-subj="resolver:node-detail:title-icon"]');
public testSubject(selector: string): ReactWrapper {
return this.domNodes(`[data-test-subj="${selector}"]`);
}

/**
Expand Down Expand Up @@ -297,7 +266,7 @@ export class Simulator {
public async resolveWrapper(
wrapperFactory: () => ReactWrapper,
predicate: (wrapper: ReactWrapper) => boolean = (wrapper) => wrapper.length > 0
): Promise<ReactWrapper | void> {
): Promise<ReactWrapper | undefined> {
for await (const wrapper of this.map(wrapperFactory)) {
if (predicate(wrapper)) {
return wrapper;
Expand Down
15 changes: 8 additions & 7 deletions x-pack/plugins/security_solution/public/resolver/test_utilities/simulator/mock_resolver.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

/* eslint-disable react/display-name */

import React, { useMemo, useEffect, useState, useCallback } from 'react';
import React, { useEffect, useState, useCallback } from 'react';
import { Router } from 'react-router-dom';
import { I18nProvider } from '@kbn/i18n/react';
import { Provider } from 'react-redux';
Expand All @@ -17,7 +17,6 @@ import { ResolverState, SideEffectSimulator, ResolverProps } from '../../types';
import { ResolverAction } from '../../store/actions';
import { ResolverWithoutProviders } from '../../view/resolver_without_providers';
import { SideEffectContext } from '../../view/side_effect_context';
import { sideEffectSimulatorFactory } from '../../view/side_effect_simulator_factory';

type MockResolverProps = {
/**
Expand All @@ -38,6 +37,10 @@ type MockResolverProps = {
history: React.ComponentProps<typeof Router>['history'];
/** Pass a resolver store. See `storeFactory` and `mockDataAccessLayer` */
store: Store<ResolverState, ResolverAction>;
/**
* Pass the side effect simulator which handles animations and resizing. See `sideEffectSimulatorFactory`
*/
sideEffectSimulator: SideEffectSimulator;
/**
* All the props from `ResolverWithoutStore` can be passed. These aren't defaulted to anything (you might want to test what happens when they aren't present.)
*/
Expand Down Expand Up @@ -66,8 +69,6 @@ export const MockResolver = React.memo((props: MockResolverProps) => {
setResolverElement(element);
}, []);

const simulator: SideEffectSimulator = useMemo(() => sideEffectSimulatorFactory(), []);

// Resize the Resolver element to match the passed in props. Resolver is size dependent.
useEffect(() => {
if (resolverElement) {
Expand All @@ -84,15 +85,15 @@ export const MockResolver = React.memo((props: MockResolverProps) => {
return this;
},
};
simulator.controls.simulateElementResize(resolverElement, size);
props.sideEffectSimulator.controls.simulateElementResize(resolverElement, size);
}
}, [props.rasterWidth, props.rasterHeight, simulator.controls, resolverElement]);
}, [props.rasterWidth, props.rasterHeight, props.sideEffectSimulator.controls, resolverElement]);

return (
<I18nProvider>
<Router history={props.history}>
<KibanaContextProvider services={props.coreStart}>
<SideEffectContext.Provider value={simulator.mock}>
<SideEffectContext.Provider value={props.sideEffectSimulator.mock}>
<Provider store={props.store}>
<ResolverWithoutProviders
ref={resolverRef}
Expand Down
37 changes: 23 additions & 14 deletions x-pack/plugins/security_solution/public/resolver/view/clickthrough.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ describe('Resolver, when analyzing a tree that has no ancestors and 2 children',
* For example, there might be no loading element at one point, and 1 graph element at one point, but never a single time when there is both 1 graph element and 0 loading elements.
*/
simulator.map(() => ({
graphElements: simulator.graphElement().length,
graphLoadingElements: simulator.graphLoadingElement().length,
graphErrorElements: simulator.graphErrorElement().length,
graphElements: simulator.testSubject('resolver:graph').length,
graphLoadingElements: simulator.testSubject('resolver:graph:loading').length,
graphErrorElements: simulator.testSubject('resolver:graph:error').length,
}))
).toYieldEqualTo({
// it should have 1 graph element, an no error or loading elements.
Expand Down Expand Up @@ -72,8 +72,12 @@ describe('Resolver, when analyzing a tree that has no ancestors and 2 children',
});

it(`should show links to the 3 nodes (with icons) in the node list.`, async () => {
await expect(simulator.map(() => simulator.nodeListNodeLinkText().length)).toYieldEqualTo(3);
await expect(simulator.map(() => simulator.nodeListNodeLinkIcons().length)).toYieldEqualTo(3);
await expect(
simulator.map(() => simulator.testSubject('resolver:node-list:node-link:title').length)
).toYieldEqualTo(3);
await expect(
Copy link
Contributor

Choose a reason for hiding this comment

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

seems this is duplicated

Copy link
Contributor

Choose a reason for hiding this comment

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

oh i see. the old version had an expect for 'text' and an expect for 'icons'

simulator.map(() => simulator.testSubject('resolver:node-list:node-link:title').length)
).toYieldEqualTo(3);
});

describe("when the second child node's first button has been clicked", () => {
Expand Down Expand Up @@ -131,9 +135,9 @@ describe('Resolver, when analyzing a tree that has two related events for the or
beforeEach(async () => {
await expect(
simulator.map(() => ({
graphElements: simulator.graphElement().length,
graphLoadingElements: simulator.graphLoadingElement().length,
graphErrorElements: simulator.graphErrorElement().length,
graphElements: simulator.testSubject('resolver:graph').length,
graphLoadingElements: simulator.testSubject('resolver:graph:loading').length,
graphErrorElements: simulator.testSubject('resolver:graph:error').length,
originNode: simulator.processNodeElements({ entityID: entityIDs.origin }).length,
}))
).toYieldEqualTo({
Expand All @@ -147,7 +151,10 @@ describe('Resolver, when analyzing a tree that has two related events for the or
it('should render a related events button', async () => {
await expect(
simulator.map(() => ({
relatedEventButtons: simulator.processNodeRelatedEventButton(entityIDs.origin).length,
relatedEventButtons: simulator.processNodeChildElements(
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats the purpose of the special treatment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as below. Retain the original intention here. I could move all of the actual selector string into this test or maybe even export the processNodeElementSelector and build the string at this level. Alternatively, I could tie the actual selector with the associated entity-id (see below comment)

entityIDs.origin,
'resolver:submenu:button'
).length,
}))
).toYieldEqualTo({
relatedEventButtons: 1,
Expand All @@ -156,33 +163,35 @@ describe('Resolver, when analyzing a tree that has two related events for the or
describe('when the related events button is clicked', () => {
beforeEach(async () => {
const button = await simulator.resolveWrapper(() =>
simulator.processNodeRelatedEventButton(entityIDs.origin)
simulator.processNodeChildElements(entityIDs.origin, 'resolver:submenu:button')
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just testSubject here? This couples the test to DOM structure. Is there a reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just doing a one to one replacement that generalized this test a bit more. The only way to really not have this tied to the structure of the DOM would be to replace the resolver:submenu:button with a reference to the entity-id, maybe resolver:node-${entityID}:submenu:button. Is that what you're suggesting?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see what you mean. I wasn't necessarily suggesting anything but here's my thoughts:

  • The user doesn't care about the DOM structure and so if we don't rely on it our tests will be less brittle.
  • It would be cool if we are only coupled to data-test attributes. That should make it simple to keep tests working. If we remove a wrapper div, I would hope that no tests break.

Maybe at our next office hours we can discuss those ideas more.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is some semantic weight to. e.g. the button being a child / ("owned by" in the case of semantics) of the process node. I say keep it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or to put it another way: If the relationship between the node and the interactive element it's expected to contain changes in some way, I think it's good that a test picks that up.

);
if (button) {
button.simulate('click');
}
});
it('should open the submenu and display exactly one option with the correct count', async () => {
await expect(
simulator.map(() => simulator.processNodeSubmenuItems().map((node) => node.text()))
simulator.map(() =>
simulator.testSubject('resolver:map:node-submenu-item').map((node) => node.text())
)
).toYieldEqualTo(['2 registry']);
await expect(
simulator.map(() => simulator.processNodeSubmenuItems().length)
simulator.map(() => simulator.testSubject('resolver:map:node-submenu-item').length)
).toYieldEqualTo(1);
});
});
describe('and when the related events button is clicked again', () => {
beforeEach(async () => {
const button = await simulator.resolveWrapper(() =>
simulator.processNodeRelatedEventButton(entityIDs.origin)
simulator.processNodeChildElements(entityIDs.origin, 'resolver:submenu:button')
);
if (button) {
button.simulate('click');
}
});
it('should close the submenu', async () => {
await expect(
simulator.map(() => simulator.processNodeSubmenuItems().length)
simulator.map(() => simulator.testSubject('resolver:map:node-submenu-item').length)
).toYieldEqualTo(0);
});
});
Expand Down
Loading