Skip to content

Commit

Permalink
Merge pull request #131 from jaegertracing/fixit-week/test-coverage
Browse files Browse the repository at this point in the history
Increase test coverage
  • Loading branch information
tiffon authored Dec 13, 2017
2 parents 85f9822 + 78cfe88 commit ffbbcea
Show file tree
Hide file tree
Showing 28 changed files with 1,941 additions and 355 deletions.
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ script:
- npm run coverage
- npm run build
after_success:
- npm install -g coveralls
- cat coverage/lcov.info | coveralls
- npm install -g codecov
- codecov
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@
"jest": {
"collectCoverageFrom": [
"src/**/*.js",
"!src/utils/DraggableManager/demo/*.js"
"!src/utils/DraggableManager/demo/*.js",
"!src/demo/**/*.js"
]
},
"prettier": {
Expand Down
8 changes: 5 additions & 3 deletions src/components/App/Page.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ type PageProps = {
config: Config,
};

class Page extends React.Component<PageProps> {
// export for tests
export class PageImpl extends React.Component<PageProps> {
props: PageProps;

componentDidMount() {
Expand Down Expand Up @@ -61,10 +62,11 @@ class Page extends React.Component<PageProps> {
}
}

function mapStateToProps(state, ownProps) {
// export for tests
export function mapStateToProps(state: { config: Config, router: { location: Location } }, ownProps: any) {
const { config } = state;
const { location } = state.router;
return { ...ownProps, config, location };
}

export default withRouter(connect(mapStateToProps)(Page));
export default withRouter(connect(mapStateToProps)(PageImpl));
71 changes: 71 additions & 0 deletions src/components/App/Page.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright (c) 2017 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

/* eslint-disable import/first */
jest.mock('./TopNav', () => () => <div />);
jest.mock('../../utils/metrics');

import React from 'react';
import { mount } from 'enzyme';

import { mapStateToProps, PageImpl as Page } from './Page';
import { trackPageView } from '../../utils/metrics';

describe('mapStateToProps()', () => {
it('maps state to props', () => {
const state = {
config: {},
router: { location: {} },
};
const ownProps = { a: {} };
expect(mapStateToProps(state, ownProps)).toEqual({
config: state.config,
location: state.router.location,
a: ownProps.a,
});
});
});

describe('<Page>', () => {
let props;
let wrapper;

beforeEach(() => {
trackPageView.mockReset();
props = {
location: {
pathname: String(Math.random()),
search: String(Math.random()),
},
config: { menu: [] },
};
wrapper = mount(<Page {...props} />);
});

it('does not explode', () => {
expect(wrapper).toBeDefined();
});

it('tracks an initial page-view', () => {
const { pathname, search } = props.location;
expect(trackPageView.mock.calls).toEqual([[pathname, search]]);
});

it('tracks a pageView when the location changes', () => {
trackPageView.mockReset();
const location = { pathname: 'le-path', search: 'searching' };
wrapper.setProps({ location });
expect(trackPageView.mock.calls).toEqual([[location.pathname, location.search]]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,23 @@
// See the License for the specific language governing permissions and
// limitations under the License.

const BLUE = '#3683bb';
const LIGHT_BLUE = '#6eafd4';
const RED = '#e45629';
const ORANGE = '#fb8d46';
const GREEN = '#37a257';
/* eslint-disable import/first */
jest.mock('cytoscape');

const colors = [BLUE, LIGHT_BLUE, RED, ORANGE, GREEN];
import React from 'react';
import { mount } from 'enzyme';

export default colors;
import DAG from './DAG';

describe('<DAG>', () => {
it('does not explode', () => {
const serviceCalls = [
{
callCount: 1,
child: 'child-id',
parent: 'parent-id',
},
];
expect(mount(<DAG serviceCalls={serviceCalls} />)).toBeDefined();
});
});
15 changes: 7 additions & 8 deletions src/components/DependencyGraph/DependencyForceGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ import { debounce } from 'lodash';

import { nodesPropTypes, linksPropTypes } from '../../propTypes/dependencies';

const chargeStrength = ({ radius = 5, orphan }) => (orphan ? -20 * radius : -12 * radius);
// export for tests
export const chargeStrength = ({ radius = 5, orphan }) => (orphan ? -20 * radius : -12 * radius);

export default class DependencyForceGraph extends Component {
static get propTypes() {
return {
nodes: nodesPropTypes.isRequired,
links: linksPropTypes.isRequired,
};
}
static propTypes = {
nodes: nodesPropTypes.isRequired,
links: linksPropTypes.isRequired,
};

constructor(props) {
super(props);
Expand All @@ -39,7 +38,7 @@ export default class DependencyForceGraph extends Component {
};
}

componentDidMount() {
componentWillMount() {
this.onResize();
this.debouncedResize = debounce((...args) => this.onResize(...args), 50);
window.addEventListener('resize', this.debouncedResize);
Expand Down
116 changes: 116 additions & 0 deletions src/components/DependencyGraph/DependencyForceGraph.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// Copyright (c) 2017 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import React from 'react';
import { shallow } from 'enzyme';
import { InteractiveForceGraph, ForceGraphNode, ForceGraphLink } from 'react-vis-force';

import DependencyForceGraph, { chargeStrength } from './DependencyForceGraph';

describe('chargeStrength', () => {
it('returns a number', () => {
expect(chargeStrength({ radius: 1, orphan: false })).toBeLessThan(0);
});

it('handles orphan as a special case', () => {
const asOrphan = chargeStrength({ radius: 1, orphan: true });
const notOrphan = chargeStrength({ radius: 1, orphan: false });
expect(chargeStrength(asOrphan)).toBeLessThan(0);
expect(chargeStrength(notOrphan)).toBeLessThan(0);
expect(asOrphan).not.toBe(notOrphan);
});
});

describe('<DependencyForceGraph>', () => {
const nodes = [{ id: 'node-a', radius: 1 }, { id: 'node-b', radius: 1 }];
const links = [{ source: 'node-a', target: 'node-b', value: 1 }];
let oldSize;
let wrapper;

beforeAll(() => {
oldSize = {
width: window.innerWidth,
height: window.innerHeight,
};
});

afterAll(() => {
const { height, width } = oldSize;
window.innerHeight = height;
window.innerWidth = width;
});

beforeEach(() => {
window.innerWidth = 1234;
window.innerHeight = 5678;
wrapper = shallow(<DependencyForceGraph nodes={nodes} links={links} />);
});

it('does not explode', () => {
expect(wrapper).toBeDefined();
expect(wrapper.length).toBe(1);
});

it('saves the window dimensions to state', () => {
const { height, width } = wrapper.state();
expect(height).toBe(window.innerHeight);
expect(width).toBe(window.innerWidth);
});

describe('window resize event', () => {
it('adds and removes an event listener on mount and unmount', () => {
const oldFns = {
addFn: window.addEventListener,
removeFn: window.removeEventListener,
};
window.addEventListener = jest.fn();
window.removeEventListener = jest.fn();
wrapper = shallow(<DependencyForceGraph nodes={nodes} links={links} />);
expect(window.addEventListener.mock.calls.length).toBe(1);
expect(window.removeEventListener.mock.calls.length).toBe(0);
wrapper.unmount();
expect(window.removeEventListener.mock.calls.length).toBe(1);
window.addEventListener = oldFns.addFn;
window.removeEventListener = oldFns.removeFn;
});

it('updates the saved window dimensions on resize', () => {
const { height: preHeight, width: preWidth } = wrapper.state();
window.innerHeight *= 2;
window.innerWidth *= 2;
// difficult to get JSDom to dispatch the window resize event, so hit
// the listener directly
wrapper.instance().onResize();
const { height, width } = wrapper.state();
expect(height).toBe(window.innerHeight);
expect(width).toBe(window.innerWidth);
expect(height).not.toBe(preHeight);
expect(width).not.toBe(preWidth);
});
});

describe('render', () => {
it('renders a InteractiveForceGraph', () => {
expect(wrapper.find(InteractiveForceGraph).length).toBe(1);
});

it('renders a <ForceGraphNode> for each node', () => {
expect(wrapper.find(ForceGraphNode).length).toBe(nodes.length);
});

it('renders a <ForceGraphLink> for each link', () => {
expect(wrapper.find(ForceGraphLink).length).toBe(links.length);
});
});
});
19 changes: 13 additions & 6 deletions src/components/DependencyGraph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ import { nodesPropTypes, linksPropTypes } from '../../propTypes/dependencies';
import DependencyForceGraph from './DependencyForceGraph';
import DAG from './DAG';

// export for tests
export const GRAPH_TYPES = {
FORCE_DIRECTED: { type: 'FORCE_DIRECTED', name: 'Force Directed Graph' },
DAG: { type: 'DAG', name: 'DAG' },
};

export default class DependencyGraphPage extends Component {
static propTypes = {
// eslint-disable-next-line react/forbid-prop-types
Expand All @@ -51,7 +57,7 @@ export default class DependencyGraphPage extends Component {
graphType: 'FORCE_DIRECTED',
};
}
componentDidMount() {
componentWillMount() {
this.props.fetchDependencies();
}

Expand Down Expand Up @@ -83,10 +89,10 @@ export default class DependencyGraphPage extends Component {
);
}

const GRAPH_TYPE_OPTIONS = [{ type: 'FORCE_DIRECTED', name: 'Force Directed Graph' }];
const GRAPH_TYPE_OPTIONS = [GRAPH_TYPES.FORCE_DIRECTED];

if (dependencies.length <= 100) {
GRAPH_TYPE_OPTIONS.push({ type: 'DAG', name: 'DAG' });
GRAPH_TYPE_OPTIONS.push(GRAPH_TYPES.DAG);
}
return (
<div className="my2">
Expand Down Expand Up @@ -119,8 +125,8 @@ export default class DependencyGraphPage extends Component {
}
}

// export connected component separately
function mapStateToProps(state) {
// export for tests
export function mapStateToProps(state) {
const { dependencies, error, loading } = state.dependencies;
let links;
let nodes;
Expand All @@ -132,7 +138,8 @@ function mapStateToProps(state) {
return { loading, error, nodes, links, dependencies };
}

function mapDispatchToProps(dispatch) {
// export for tests
export function mapDispatchToProps(dispatch) {
const { fetchDependencies } = bindActionCreators(jaegerApiActions, dispatch);
return { fetchDependencies };
}
Expand Down
Loading

0 comments on commit ffbbcea

Please sign in to comment.