From 005cf0f3d6f2482b62535401b2eb0ed76b3e0ea7 Mon Sep 17 00:00:00 2001 From: Tim Klever Date: Fri, 21 Aug 2020 18:22:42 -0700 Subject: [PATCH] Additional Test Coverage around TimelineViewingLayer (#617) * add additional tests around existing TimelineViewingLayer functionality In addition to rounding out the test suite, this commit also removes some of the "Math.random" calls from the test suite. These randoms were causing some code coverage jitter in the test suite by randomly executing certain code paths. Removing these randomizations will keep coverage reporting consistent and avoid random "coverage losses" based on chance. Signed-off-by: Tim Klever * migrate from callback ref to CreateRef in TimelineViewingLayer CreateRef API was introduced in React 16.3 Signed-off-by: Tim Klever Co-authored-by: Ruben Vargas Palma Signed-off-by: vvvprabhakar --- .../TimelineViewingLayer.test.js | 50 +++++++++++++++++-- .../TimelineViewingLayer.tsx | 29 ++++++----- 2 files changed, 60 insertions(+), 19 deletions(-) diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.test.js b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.test.js index 9b14704197..e909aa6207 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.test.js +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.test.js @@ -50,7 +50,7 @@ describe('', () => { it('sets _root to the root DOM node', () => { expect(instance._root).toBeDefined(); - expect(wrapper.find('.TimelineViewingLayer').getDOMNode()).toBe(instance._root); + expect(wrapper.find('.TimelineViewingLayer').getDOMNode()).toBe(instance._root.current); }); describe('uses DraggableManager', () => { @@ -75,10 +75,17 @@ describe('', () => { it('returns the dragging bounds from _getDraggingBounds()', () => { const left = 10; const width = 100; - instance._root.getBoundingClientRect = () => ({ left, width }); + instance._root.current.getBoundingClientRect = () => ({ left, width }); expect(instance._getDraggingBounds()).toEqual({ width, clientXLeft: left }); }); + it('throws error on call to _getDraggingBounds() on unmounted component', () => { + wrapper.unmount(); + expect(instance._getDraggingBounds).toThrow( + 'Component must be mounted in order to determine DraggableBounds' + ); + }); + it('updates viewRange.time.cursor via _draggerReframe._onMouseMove', () => { const value = 0.5; const cursor = mapFromSubRange(viewStart, viewEnd, value); @@ -101,7 +108,7 @@ describe('', () => { it('handles drag move via _draggerReframe._onDragMove', () => { const anchor = 0.25; - const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor, shift: Math.random() } }; + const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor, shift: 0.5 } }; const value = 0.5; const shift = mapFromSubRange(viewStart, viewEnd, value); // make sure `anchor` is already present on the props @@ -118,12 +125,29 @@ describe('', () => { const value = 0.5; const shift = mapFromSubRange(viewStart, viewEnd, value); const anchor = 0.25; - const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor, shift: Math.random() } }; + const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor, shift } }; wrapper.setProps({ viewRangeTime }); instance._draggerReframe._onDragEnd({ manager, value }); expect(manager.resetBounds.mock.calls).toEqual([[]]); expect(props.updateViewRangeTime.mock.calls).toEqual([[anchor, shift, 'timeline-header']]); }); + + it('_draggerReframe._onDragEnd sorts anchor and shift', () => { + const manager = { resetBounds: jest.fn() }; + const value = 0.5; + const shift = mapFromSubRange(viewStart, viewEnd, value); + const anchor = 0.75; + const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor, shift } }; + wrapper.setProps({ viewRangeTime }); + instance._draggerReframe._onDragEnd({ manager, value }); + expect(props.updateViewRangeTime.mock.calls).toEqual([[shift, anchor, 'timeline-header']]); + }); + + it('resets draggable bounds on boundsInvalidator update', () => { + const spy = jest.spyOn(instance._draggerReframe, 'resetBounds'); + wrapper.setProps({ boundsInvalidator: 'SOMETHING-NEW' }); + expect(spy).toHaveBeenCalledTimes(1); + }); }); describe('render()', () => { @@ -155,6 +179,24 @@ describe('', () => { expect(wrapper.find('.isDraggingRight.isReframeDrag').length).toBe(1); }); + it('renders the reframe dragging normalized left', () => { + const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor: -0.25, shift: viewEnd } }; + wrapper.setProps({ viewRangeTime }); + expect(wrapper.find('.isDraggingRight.isReframeDrag').length).toBe(1); + }); + + it('renders the reframe dragging normalized right', () => { + const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor: viewStart, shift: 1.25 } }; + wrapper.setProps({ viewRangeTime }); + expect(wrapper.find('.isDraggingRight.isReframeDrag').length).toBe(1); + }); + + it('does not render the reframe on out of bounds', () => { + const viewRangeTime = { ...props.viewRangeTime, reframe: { anchor: 1.5, shift: 1.75 } }; + wrapper.setProps({ viewRangeTime }); + expect(wrapper.find('.isReframeDrag').length).toBe(0); + }); + it('renders the shiftStart dragging', () => { const viewRangeTime = { ...props.viewRangeTime, shiftStart: viewEnd }; wrapper.setProps({ viewRangeTime }); diff --git a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.tsx b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.tsx index e5eaf0a937..7b1622fcb5 100644 --- a/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.tsx +++ b/packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.tsx @@ -122,7 +122,7 @@ function getMarkers( */ export default class TimelineViewingLayer extends React.PureComponent { _draggerReframe: DraggableManager; - _root: Element | TNil; + _root: React.RefObject; constructor(props: TimelineViewingLayerProps) { super(props); @@ -134,7 +134,7 @@ export default class TimelineViewingLayer extends React.PureComponent) { @@ -148,15 +148,12 @@ export default class TimelineViewingLayer extends React.PureComponent { - this._root = elm; - }; - _getDraggingBounds = (): DraggableBounds => { - if (!this._root) { - throw new Error('invalid state'); + const current = this._root.current; + if (!current) { + throw new Error('Component must be mounted in order to determine DraggableBounds'); } - const { left: clientXLeft, width } = this._root.getBoundingClientRect(); + const { left: clientXLeft, width } = current.getBoundingClientRect(); return { clientXLeft, width }; }; @@ -170,20 +167,22 @@ export default class TimelineViewingLayer extends React.PureComponent { + _getAnchorAndShift = (value: number) => { const { current, reframe } = this.props.viewRangeTime; const [viewStart, viewEnd] = current; const shift = mapFromViewSubRange(viewStart, viewEnd, value); const anchor = reframe ? reframe.anchor : shift; + return { anchor, shift }; + }; + + _handleReframeDragUpdate = ({ value }: DraggingUpdate) => { + const { anchor, shift } = this._getAnchorAndShift(value); const update = { reframe: { anchor, shift } }; this.props.updateNextViewRangeTime(update); }; _handleReframeDragEnd = ({ manager, value }: DraggingUpdate) => { - const { current, reframe } = this.props.viewRangeTime; - const [viewStart, viewEnd] = current; - const shift = mapFromViewSubRange(viewStart, viewEnd, value); - const anchor = reframe ? reframe.anchor : shift; + const { anchor, shift } = this._getAnchorAndShift(value); const [start, end] = shift < anchor ? [shift, anchor] : [anchor, shift]; manager.resetBounds(); this.props.updateViewRangeTime(start, end, 'timeline-header'); @@ -202,7 +201,7 @@ export default class TimelineViewingLayer extends React.PureComponent