Skip to content

Commit

Permalink
Additional Test Coverage around TimelineViewingLayer (jaegertracing#617)
Browse files Browse the repository at this point in the history 
* 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 <Tim.V.Klever@aexp.com>

* migrate from callback ref to CreateRef in TimelineViewingLayer

CreateRef API was introduced in React 16.3

Signed-off-by: Tim Klever <Tim.V.Klever@aexp.com>

Co-authored-by: Ruben Vargas Palma <ruben.vp8510@gmail.com>
Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
  • Loading branch information
@tklever @rubenvp8510
tklever and rubenvp8510 committed Aug 22, 2020
1 parent a4adf7e commit 6d0c356
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 19 deletions.
50 changes: 46 additions & 4 deletions ...c/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('<TimelineViewingLayer>', () => {

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', () => {
Expand All @@ -75,10 +75,17 @@ describe('<TimelineViewingLayer>', () => {
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);
Expand All @@ -101,7 +108,7 @@ describe('<TimelineViewingLayer>', () => {

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
Expand All @@ -118,12 +125,29 @@ describe('<TimelineViewingLayer>', () => {
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()', () => {
Expand Down Expand Up @@ -155,6 +179,24 @@ describe('<TimelineViewingLayer>', () => {
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 });
Expand Down
29 changes: 14 additions & 15 deletions ...i/src/components/TracePage/TraceTimelineViewer/TimelineHeaderRow/TimelineViewingLayer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ function getMarkers(
*/
export default class TimelineViewingLayer extends React.PureComponent<TimelineViewingLayerProps> {
_draggerReframe: DraggableManager;
_root: Element | TNil;
_root: React.RefObject<HTMLDivElement>;

constructor(props: TimelineViewingLayerProps) {
super(props);
Expand All @@ -134,7 +134,7 @@ export default class TimelineViewingLayer extends React.PureComponent<TimelineVi
onMouseLeave: this._handleReframeMouseLeave,
onMouseMove: this._handleReframeMouseMove,
});
this._root = undefined;
this._root = React.createRef();
}

componentDidUpdate(prevProps: Readonly<TimelineViewingLayerProps>) {
Expand All @@ -148,15 +148,12 @@ export default class TimelineViewingLayer extends React.PureComponent<TimelineVi
this._draggerReframe.dispose();
}

_setRoot = (elm: Element | TNil) => {
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 };
};

Expand All @@ -170,20 +167,22 @@ export default class TimelineViewingLayer extends React.PureComponent<TimelineVi
this.props.updateNextViewRangeTime({ cursor: undefined });
};

_handleReframeDragUpdate = ({ value }: DraggingUpdate) => {
_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');
Expand All @@ -202,7 +201,7 @@ export default class TimelineViewingLayer extends React.PureComponent<TimelineVi
<div
aria-hidden
className="TimelineViewingLayer"
ref={this._setRoot}
ref={this._root}
onMouseDown={this._draggerReframe.handleMouseDown}
onMouseLeave={this._draggerReframe.handleMouseLeave}
onMouseMove={this._draggerReframe.handleMouseMove}
Expand Down

0 comments on commit 6d0c356

Please sign in to comment.