From 8502c7e74a2e0489f00cfe93a31a35aae7ff0cd6 Mon Sep 17 00:00:00 2001 From: Everett Ross Date: Fri, 22 Nov 2019 15:07:17 -0500 Subject: [PATCH] Track set/clear service/operation with tests Signed-off-by: Everett Ross --- .../__snapshots__/index.test.js.snap | 15 +++- .../Graph/DdgNodeContent/index.test.js | 83 ++++++++++++++++--- .../Graph/DdgNodeContent/index.tsx | 11 ++- .../Header/__snapshots__/index.test.js.snap | 4 +- .../DeepDependencies/Header/index.test.js | 24 +++++- .../DeepDependencies/Header/index.tsx | 9 +- .../components/DeepDependencies/index.test.js | 16 ++++ .../DeepDependencies/index.track.tsx | 20 +++++ .../src/components/DeepDependencies/index.tsx | 4 +- 9 files changed, 164 insertions(+), 22 deletions(-) diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap index e09fde8198..2fcb38b9c8 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/__snapshots__/index.test.js.snap @@ -57,7 +57,19 @@ exports[` DdgNodeContent.getNodeRenderer() returns a renders the number of operations if there are multiple "op3", ] } + setValue={[Function]} value={null} /> } diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js index 494d6b1438..d3282587ec 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.test.js @@ -22,10 +22,12 @@ jest.mock('./calc-positioning', () => () => ({ /* eslint-disable import/first */ import React from 'react'; import { shallow } from 'enzyme'; -import { Checkbox } from 'antd'; +import { Checkbox, Popover } from 'antd'; import DdgNodeContent from '.'; import { MAX_LENGTH, MAX_LINKED_TRACES, MIN_LENGTH, PARAM_NAME_LENGTH, RADIUS } from './constants'; +import * as track from '../../index.track'; +import FilteredList from '../../../common/FilteredList'; import * as getSearchUrl from '../../../SearchTracePage/url'; import { ECheckedStatus, EDdgDensity, EDirection, EViewModifier } from '../../../../model/ddg/types'; @@ -34,6 +36,7 @@ describe('', () => { const vertexKey = 'some-key'; const service = 'some-service'; const operation = 'some-operation'; + const operationArray = ['op0', 'op1', 'op2', 'op3']; const props = { focalNodeUrl: 'some-url', focusPathsThroughVertex: jest.fn(), @@ -42,6 +45,7 @@ describe('', () => { hideVertex: jest.fn(), isFocalNode: false, operation, + setOperation: jest.fn(), setViewModifier: jest.fn(), service, updateGenerationVisibility: jest.fn(), @@ -51,6 +55,7 @@ describe('', () => { let wrapper; beforeEach(() => { + props.getGenerationVisibility.mockReturnValue(null).mockClear(); props.getVisiblePathElems.mockReset(); props.setViewModifier.mockReset(); props.updateGenerationVisibility.mockReset(); @@ -69,7 +74,7 @@ describe('', () => { it('renders the number of operations if there are multiple', () => { expect(wrapper).toMatchSnapshot(); - wrapper.setProps({ operation: ['op0', 'op1', 'op2', 'op3'] }); + wrapper.setProps({ operation: operationArray }); expect(wrapper).toMatchSnapshot(); }); @@ -158,6 +163,17 @@ describe('', () => { parentVisibility, }); }); + + it('handles mouse over event after vis update would hide vertex before unmounting', () => { + props.getVisiblePathElems.mockReturnValue(undefined); + wrapper.simulate('mouseover', { type: 'mouseover' }); + + expect(wrapper.state()).toEqual({ + childrenVisibility: null, + parentVisibility: null, + }); + expect(props.setViewModifier).toHaveBeenCalledWith([], EViewModifier.Hovered, true); + }); }); describe('node interactions', () => { @@ -193,6 +209,41 @@ describe('', () => { }); }); + describe('setFocus', () => { + let trackSetFocusSpy; + + beforeAll(() => { + trackSetFocusSpy = jest.spyOn(track, 'trackSetFocus'); + }); + + it('tracks when setFocus link is clicked', () => { + expect(trackSetFocusSpy).toHaveBeenCalledTimes(0); + wrapper.find(`[href="${props.focalNodeUrl}"]`).simulate('click'); + expect(trackSetFocusSpy).toHaveBeenCalledTimes(1); + }); + }); + + describe('setOperation', () => { + let trackSetOpSpy; + + beforeAll(() => { + trackSetOpSpy = jest.spyOn(track, 'trackVertexSetOperation'); + }); + + it('tracks when popover sets a value', () => { + wrapper.setProps({ operation: operationArray }); + expect(trackSetOpSpy).not.toHaveBeenCalled(); + + const list = shallow(
{wrapper.find(Popover).prop('content')}
).find(FilteredList); + expect(list.prop('options')).toBe(operationArray); + expect(trackSetOpSpy).not.toHaveBeenCalled(); + + list.prop('setValue')(operationArray[operationArray.length - 1]); + expect(props.setOperation).toHaveBeenCalledWith(operationArray[operationArray.length - 1]); + expect(trackSetOpSpy).toHaveBeenCalledTimes(1); + }); + }); + describe('updateChildren', () => { it('renders children visibility status indicator iff state.childrenVisibility is provided', () => { const initialItemCount = wrapper.find('.DdgNodeContent--actionsItem').length; @@ -320,17 +371,27 @@ describe('', () => { return ids; }; let getSearchUrlSpy; - const lastIDs = () => getSearchUrlSpy.mock.calls[getSearchUrlSpy.mock.calls.length - 1][0].traceID; + let trackViewTracesSpy; + const verifyLastIDs = expectedIDs => { + const getSearchUrlCallCount = getSearchUrlSpy.mock.calls.length; + const actualIDs = getSearchUrlSpy.mock.calls[getSearchUrlCallCount - 1][0].traceID; + + expect(actualIDs.sort()).toEqual(expectedIDs.sort()); + expect(trackViewTracesSpy).toHaveBeenCalledTimes(getSearchUrlCallCount); + }; let originalOpen; beforeAll(() => { originalOpen = window.open; window.open = jest.fn(); getSearchUrlSpy = jest.spyOn(getSearchUrl, 'getUrl'); + trackViewTracesSpy = jest.spyOn(track, 'trackViewTraces'); }); beforeEach(() => { + getSearchUrlSpy.mockClear(); window.open.mockReset(); + trackViewTracesSpy.mockReset(); }); afterAll(() => { @@ -347,7 +408,7 @@ describe('', () => { const ids = makeIDsAndMock([1]); click(); - expect(lastIDs().sort()).toEqual([].concat(...ids).sort()); + verifyLastIDs([].concat(...ids)); expect(props.getVisiblePathElems).toHaveBeenCalledTimes(1); expect(props.getVisiblePathElems).toHaveBeenCalledWith(vertexKey); }); @@ -356,14 +417,14 @@ describe('', () => { const ids = makeIDsAndMock([3]); click(); - expect(lastIDs().sort()).toEqual([].concat(...ids).sort()); + verifyLastIDs([].concat(...ids)); }); it('opens new tab viewing multiple traceIDs from multiple elems', () => { const ids = makeIDsAndMock([3, 2]); click(); - expect(lastIDs().sort()).toEqual([].concat(...ids).sort()); + verifyLastIDs([].concat(...ids)); }); it('ignores falsy and duplicate IDs', () => { @@ -371,7 +432,7 @@ describe('', () => { falsifyDuplicateAndMock(ids); click(); - expect(lastIDs().sort()).toEqual([].concat(...ids).sort()); + verifyLastIDs([].concat(...ids)); }); describe('MAX_LINKED_TRACES', () => { @@ -386,14 +447,14 @@ describe('', () => { mockReturn(ids); click(); - expect(lastIDs().sort()).toEqual(expected); + verifyLastIDs(expected); }); it('does not count falsy and duplicate IDs towards MAX_LINKED_TRACES', () => { falsifyDuplicateAndMock(ids); click(); - expect(lastIDs().sort()).toEqual(expected); + verifyLastIDs(expected); }); }); @@ -413,14 +474,14 @@ describe('', () => { mockReturn(ids); click(); - expect(lastIDs().sort()).toEqual(expected); + verifyLastIDs(expected); }); it('does not count falsy and duplicate IDs towards MAX_LEN', () => { falsifyDuplicateAndMock(ids); click(); - expect(lastIDs().sort()).toEqual(expected); + verifyLastIDs(expected); }); }); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx index d90c092538..e901a5d88e 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Graph/DdgNodeContent/index.tsx @@ -30,7 +30,7 @@ import { WORD_RX, } from './constants'; import { setFocusIcon } from './node-icons'; -import { trackSetFocus, trackViewTraces } from '../../index.track'; +import { trackSetFocus, trackViewTraces, trackVertexSetOperation } from '../../index.track'; import { getUrl } from '../../url'; import BreakableText from '../../../common/BreakableText'; import FilteredList from '../../../common/FilteredList'; @@ -146,6 +146,11 @@ export default class DdgNodeContent extends React.PureComponent hideVertex(vertexKey); }; + private setOperation = (operation: string) => { + trackVertexSetOperation(); + this.props.setOperation(operation); + }; + private updateChildren = () => { const { updateGenerationVisibility, vertexKey } = this.props; updateGenerationVisibility(vertexKey, EDirection.Downstream); @@ -209,7 +214,7 @@ export default class DdgNodeContent extends React.PureComponent render() { const { childrenVisibility, parentVisibility } = this.state; - const { focalNodeUrl, isFocalNode, isPositioned, operation, service, setOperation } = this.props; + const { focalNodeUrl, isFocalNode, isPositioned, operation, service } = this.props; const { radius, svcWidth, opWidth, svcMarginTop } = calcPositioning(service, operation); const scaleFactor = RADIUS / radius; @@ -243,7 +248,7 @@ export default class DdgNodeContent extends React.PureComponent cancel={() => {}} options={operation} value={null} - setValue={setOperation} + setValue={this.setOperation} /> } placement="bottom" diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/index.test.js.snap b/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/index.test.js.snap index 08285a3477..a7583e9c99 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/index.test.js.snap +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/__snapshots__/index.test.js.snap @@ -57,7 +57,7 @@ exports[`
renders the hops selector if distanceToPathElems is provided 1
`; -exports[`
renders the operation selector if a service is selected 1`] = ` +exports[`
renders the operation selector iff a service is selected 1`] = `
@@ -124,7 +124,7 @@ exports[`
renders the operation selector if a service is selected 1`] =
`; -exports[`
renders the operation selector if a service is selected 2`] = ` +exports[`
renders the operation selector iff a service is selected 2`] = `
diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.test.js index 0b31eb669a..dbf037f106 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.test.js @@ -21,20 +21,26 @@ import MdVisibilityOff from 'react-icons/lib/md/visibility-off'; import Header from './index'; import HopsSelector from './HopsSelector'; import NameSelector from './NameSelector'; +import * as track from '../index.track'; describe('
', () => { const minProps = { clearOperation: () => {}, setDistance: () => {}, - setOperation: () => {}, + setOperation: jest.fn(), setService: () => {}, }; const service = 'testService'; const services = [service]; const operation = 'testOperation'; const operations = [operation]; + let trackSetOpSpy; let wrapper; + beforeAll(() => { + trackSetOpSpy = jest.spyOn(track, 'trackHeaderSetOperation'); + }); + beforeEach(() => { wrapper = shallow(
); }); @@ -49,8 +55,9 @@ describe('
', () => { expect(nameSelector.prop('label')).toMatch(/service/i); }); - it('renders the operation selector if a service is selected', () => { + it('renders the operation selector iff a service is selected', () => { let nameSelector = wrapper.find(NameSelector); + expect(nameSelector.length).toBe(1); wrapper.setProps({ service, services }); nameSelector = wrapper.find(NameSelector); expect(nameSelector.length).toBe(2); @@ -61,6 +68,19 @@ describe('
', () => { expect(wrapper).toMatchSnapshot(); }); + it('tracks when operation selector sets a value', () => { + wrapper.setProps({ service, services }); + const testOp = 'test operation'; + expect(trackSetOpSpy).not.toHaveBeenCalled(); + + wrapper + .find(NameSelector) + .at(1) + .prop('setValue')(testOp); + expect(trackSetOpSpy).toHaveBeenCalledTimes(1); + expect(minProps.setOperation).toHaveBeenCalledWith(testOp); + }); + it('renders the hops selector if distanceToPathElems is provided', () => { wrapper.setProps({ distanceToPathElems: new Map(), diff --git a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx index fa4697c1bc..e306a39eda 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/Header/index.tsx @@ -20,7 +20,7 @@ import MdVisibilityOff from 'react-icons/lib/md/visibility-off'; import HopsSelector from './HopsSelector'; import NameSelector from './NameSelector'; import LayoutSettings from './LayoutSettings'; -import { trackFilter, trackShowMatches } from '../index.track'; +import { trackFilter, trackHeaderSetOperation, trackShowMatches } from '../index.track'; import UiFindInput from '../../common/UiFindInput'; import { EDirection, TDdgDistanceToPathElems, EDdgDensity } from '../../../model/ddg/types'; @@ -98,6 +98,11 @@ export default class Header extends React.PureComponent { ); }; + setOperation = (operation: string) => { + trackHeaderSetOperation(); + this.props.setOperation(operation); + }; + handleInfoClick = () => { trackShowMatches(); const { hiddenUiFindMatches, showVertices } = this.props; @@ -141,7 +146,7 @@ export default class Header extends React.PureComponent { label="Operation" placeholder="Filter by operation…" value={operation || null} - setValue={setOperation} + setValue={this.setOperation} options={operations || []} /> )} diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js index cd3974bb8a..44dd3dcd48 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.test.js +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.test.js @@ -179,10 +179,17 @@ describe('DeepDependencyGraphPage', () => { }); describe('clearOperation', () => { + let trackClearOperationSpy; + + beforeAll(() => { + trackClearOperationSpy = jest.spyOn(track, 'trackClearOperation'); + }); + it('removes op from urlState', () => { ddgPageImpl.clearOperation(); const { operation: _o, ...urlStateWithoutOp } = props.urlState; expect(getUrlSpy).toHaveBeenLastCalledWith(urlStateWithoutOp, undefined); + expect(trackClearOperationSpy).toHaveBeenCalledTimes(1); }); }); @@ -329,9 +336,15 @@ describe('DeepDependencyGraphPage', () => { describe('setService', () => { const service = 'newService'; + let trackSetServiceSpy; + + beforeAll(() => { + trackSetServiceSpy = jest.spyOn(track, 'trackSetService'); + }); beforeEach(() => { props.fetchServiceOperations.mockReset(); + trackSetServiceSpy.mockClear(); }); it('updates service and clears operation and visEncoding', () => { @@ -341,12 +354,14 @@ describe('DeepDependencyGraphPage', () => { undefined ); expect(props.history.push).toHaveBeenCalledTimes(1); + expect(trackSetServiceSpy).toHaveBeenCalledTimes(1); }); it('fetches operations for service when not yet provided', () => { ddgPageImpl.setService(service); expect(props.fetchServiceOperations).toHaveBeenLastCalledWith(service); expect(props.fetchServiceOperations).toHaveBeenCalledTimes(1); + expect(trackSetServiceSpy).toHaveBeenCalledTimes(1); const pageWithOpForService = new DeepDependencyGraphPageImpl({ ...props, @@ -355,6 +370,7 @@ describe('DeepDependencyGraphPage', () => { const { length: callCount } = props.fetchServiceOperations.mock.calls; pageWithOpForService.setService(service); expect(props.fetchServiceOperations).toHaveBeenCalledTimes(callCount); + expect(trackSetServiceSpy).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.track.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.track.tsx index 7473a558d6..5f298095a6 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.track.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.track.tsx @@ -23,12 +23,14 @@ export const CATEGORY_DOWNSTREAM_HOPS_CHANGE = 'jaeger/ux/ddg/downstream-hops-ch export const CATEGORY_DOWNSTREAM_HOPS_SELECTION = 'jaeger/ux/ddg/downstream-hops-selection'; export const CATEGORY_FILTER = 'jaeger/ux/ddg/filter'; export const CATEGORY_MATCH_INTERACTIONS = 'jaeger/ux/ddg/match-interactions'; +export const CATEGORY_SEARCH = 'jaeger/ux/ddg/search'; export const CATEGORY_TOGGLE_SHOW_OP = 'jaeger/ux/ddg/toggle-show-op'; export const CATEGORY_UPSTREAM_HOPS_CHANGE = 'jaeger/ux/ddg/upstream-hops-change'; export const CATEGORY_UPSTREAM_HOPS_SELECTION = 'jaeger/ux/ddg/upstream-hops-selection'; export const CATEGORY_VERTEX_INTERACTIONS = 'jaeger/ux/ddg/vertex-interactions'; // export for tests +export const ACTION_CLEAR_OPERATION = 'clear-operation'; export const ACTION_DECREASE = 'decrease'; export const ACTION_FOCUS_PATHS = 'focus-paths'; export const ACTION_HIDE = 'hide'; @@ -36,11 +38,17 @@ export const ACTION_HIDE_CHILDREN = 'hide-children'; export const ACTION_HIDE_PARENTS = 'hide-parents'; export const ACTION_INCREASE = 'increase'; export const ACTION_SET_FOCUS = 'set-focus'; +export const ACTION_SET_OPERATION = 'set-operation'; +export const ACTION_SET_SERVICE = 'set-service'; export const ACTION_SHOW = 'show'; export const ACTION_SHOW_CHILDREN = 'show-children'; export const ACTION_SHOW_PARENTS = 'show-parents'; export const ACTION_VIEW_TRACES = 'view-traces'; +export function trackClearOperation() { + trackEvent(CATEGORY_SEARCH, ACTION_CLEAR_OPERATION); +} + export function trackDensityChange( prevDensity: EDdgDensity, nextDensity: EDdgDensity, @@ -75,6 +83,10 @@ export function trackFocusPaths() { trackEvent(CATEGORY_VERTEX_INTERACTIONS, ACTION_FOCUS_PATHS); } +export function trackHeaderSetOperation() { + trackEvent(CATEGORY_SEARCH, ACTION_SET_OPERATION); +} + export function trackHide(direction?: EDirection) { if (!direction) { trackEvent(CATEGORY_VERTEX_INTERACTIONS, ACTION_HIDE); @@ -116,6 +128,10 @@ export function trackSetFocus() { trackEvent(CATEGORY_VERTEX_INTERACTIONS, ACTION_SET_FOCUS); } +export function trackSetService() { + trackEvent(CATEGORY_SEARCH, ACTION_SET_SERVICE); +} + export function trackShowMatches() { trackEvent(CATEGORY_MATCH_INTERACTIONS, ACTION_SHOW); } @@ -125,6 +141,10 @@ export function trackToggleShowOp(value: boolean) { trackEvent(CATEGORY_TOGGLE_SHOW_OP, action); } +export function trackVertexSetOperation() { + trackEvent(CATEGORY_VERTEX_INTERACTIONS, ACTION_SET_OPERATION); +} + export function trackViewTraces() { trackEvent(CATEGORY_VERTEX_INTERACTIONS, ACTION_VIEW_TRACES); } diff --git a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx index 75f6b0b693..59d739bbca 100644 --- a/packages/jaeger-ui/src/components/DeepDependencies/index.tsx +++ b/packages/jaeger-ui/src/components/DeepDependencies/index.tsx @@ -18,7 +18,7 @@ import _get from 'lodash/get'; import { bindActionCreators, Dispatch } from 'redux'; import { connect } from 'react-redux'; -import { trackFocusPaths, trackHide, trackShow } from './index.track'; +import { trackClearOperation, trackFocusPaths, trackHide, trackSetService, trackShow } from './index.track'; import Header from './Header'; import Graph from './Graph'; import { getUrl, getUrlState, sanitizeUrlState, ROUTE_PATH } from './url'; @@ -113,6 +113,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { } clearOperation = () => { + trackClearOperation(); this.updateUrlState({ operation: undefined }); }; @@ -184,6 +185,7 @@ export class DeepDependencyGraphPageImpl extends React.PureComponent { fetchServiceOperations(service); } this.updateUrlState({ operation: undefined, service, visEncoding: undefined }); + trackSetService(); }; setViewModifier = (visibilityIndices: number[], viewModifier: EViewModifier, enable: boolean) => {