Skip to content

Commit

Permalink
pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Yasser Elsayed committed Nov 15, 2018
1 parent 9258a70 commit e0355de
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 25 deletions.
58 changes: 36 additions & 22 deletions frontend/src/pages/RunList.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ import { shallow } from 'enzyme';

describe('RunList', () => {
const onErrorSpy = jest.fn();
const listRunsMock = jest.spyOn(Apis.runServiceApi, 'listRuns');
const getRunMock = jest.spyOn(Apis.runServiceApi, 'getRun');
const getPipelineMock = jest.spyOn(Apis.pipelineServiceApi, 'getPipeline');
const getExperimentMock = jest.spyOn(Apis.experimentServiceApi, 'getExperiment');
const listRunsSpy = jest.spyOn(Apis.runServiceApi, 'listRuns');
const getRunSpy = jest.spyOn(Apis.runServiceApi, 'getRun');
const getPipelineSpy = jest.spyOn(Apis.pipelineServiceApi, 'getPipeline');
const getExperimentSpy = jest.spyOn(Apis.experimentServiceApi, 'getExperiment');

function generateProps(): RunListProps {
return {
Expand All @@ -41,8 +41,8 @@ describe('RunList', () => {
};
}

function mockNRuns(n: number, runTemplate: Partial<ApiRunDetail>) {
getRunMock.mockImplementation(id => Promise.resolve(
function mockNRuns(n: number, runTemplate: Partial<ApiRunDetail>): void {
getRunSpy.mockImplementation(id => Promise.resolve(
produce(runTemplate, draft => {
draft.pipeline_runtime = draft.pipeline_runtime || { workflow_manifest: '' };
draft.run = draft.run || {};
Expand All @@ -51,23 +51,23 @@ describe('RunList', () => {
})
));

listRunsMock.mockImplementation(() => Promise.resolve({
listRunsSpy.mockImplementation(() => Promise.resolve({
runs: range(1, n + 1).map(i => ({
id: 'testrun' + i,
name: 'run with id: testrun' + i,
} as ApiRun)),
}));

getPipelineMock.mockImplementation(() => ({ name: 'some pipeline' }));
getExperimentMock.mockImplementation(() => ({ name: 'some experiment' }));
getPipelineSpy.mockImplementation(() => ({ name: 'some pipeline' }));
getExperimentSpy.mockImplementation(() => ({ name: 'some experiment' }));
}

beforeEach(() => {
onErrorSpy.mockClear();
listRunsMock.mockClear();
getRunMock.mockClear();
getPipelineMock.mockClear();
getExperimentMock.mockClear();
listRunsSpy.mockClear();
getRunSpy.mockClear();
getPipelineSpy.mockClear();
getExperimentSpy.mockClear();
});

it('renders the empty experience', () => {
Expand All @@ -89,12 +89,13 @@ describe('RunList', () => {
const props = generateProps();
const tree = TestUtils.mountWithRouter(<RunList {...props} />);
await (tree.instance() as RunList).refresh();
expect(Apis.runServiceApi.listRuns).toHaveBeenCalledTimes(2);
expect(Apis.runServiceApi.listRuns).toHaveBeenLastCalledWith('', 10, RunSortKeys.CREATED_AT + ' desc', undefined, undefined);
expect(props.onError).not.toHaveBeenCalled();
expect(tree).toMatchSnapshot();
});

it('loads multiple run', async () => {
it('loads multiple runs', async () => {
mockNRuns(5, {});
const props = generateProps();
const tree = shallow(<RunList {...props} />);
Expand All @@ -121,7 +122,7 @@ describe('RunList', () => {

it('displays error in run row if pipeline could not be fetched', async () => {
mockNRuns(1, { run: { pipeline_spec: { pipeline_id: 'test-pipeline-id' } } });
TestUtils.makeErrorResponseOnce(getPipelineMock, 'bad stuff happened');
TestUtils.makeErrorResponseOnce(getPipelineSpy, 'bad stuff happened');
const props = generateProps();
const tree = shallow(<RunList {...props} />);
await (tree.instance() as any)._loadRuns({});
Expand All @@ -136,7 +137,7 @@ describe('RunList', () => {
}]
}
});
TestUtils.makeErrorResponseOnce(getExperimentMock, 'bad stuff happened');
TestUtils.makeErrorResponseOnce(getExperimentSpy, 'bad stuff happened');
const props = generateProps();
const tree = shallow(<RunList {...props} />);
await (tree.instance() as any)._loadRuns({});
Expand Down Expand Up @@ -220,7 +221,7 @@ describe('RunList', () => {

it('shows pipeline name', async () => {
mockNRuns(1, { run: { pipeline_spec: { pipeline_id: 'test-pipeline-id' } } });
getPipelineMock.mockImplementationOnce(() => ({ name: 'test pipeline' }));
getPipelineSpy.mockImplementationOnce(() => ({ name: 'test pipeline' }));
const props = generateProps();
const tree = shallow(<RunList {...props} />);
await (tree.instance() as any)._loadRuns({});
Expand All @@ -236,9 +237,7 @@ describe('RunList', () => {
}]
}
});
getExperimentMock.mockImplementationOnce(() => {
return ({ name: 'test experiment' });
});
getExperimentSpy.mockImplementationOnce(() => ({ name: 'test experiment' }));
const props = generateProps();
const tree = shallow(<RunList {...props} />);
await (tree.instance() as any)._loadRuns({});
Expand Down Expand Up @@ -303,6 +302,21 @@ describe('RunList', () => {
expect(noMetricTree).toMatchSnapshot();
});

it('renders a metric container when a percentage metric has value of zero', () => {
const noMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer(
{ metric: { number_value: 0, format: RunMetricFormat.PERCENTAGE } }));
expect(noMetricTree).toMatchSnapshot();
});

it('renders a metric container when a raw metric has value of zero', () => {
const noMetricTree = shallow((RunList.prototype as any)._metricCustomRenderer(
{
metadata: { maxValue: 10, minValue: 0 },
metric: { number_value: 0, format: RunMetricFormat.RAW },
}));
expect(noMetricTree).toMatchSnapshot();
});

it('renders percentage metric', () => {
const tree = shallow((RunList.prototype as any)._metricCustomRenderer(
{ metric: { number_value: 0.3, format: RunMetricFormat.PERCENTAGE } as ApiRunMetric }));
Expand All @@ -327,7 +341,7 @@ describe('RunList', () => {
expect(tree).toMatchSnapshot();
});

it('renders raw metric that is less than its min value', () => {
it('renders raw metric that is less than its min value, logs error to console', () => {
const consoleSpy = jest.spyOn(console, 'error').mockImplementation();
const tree = shallow((RunList.prototype as any)._metricCustomRenderer(
{
Expand All @@ -338,7 +352,7 @@ describe('RunList', () => {
expect(consoleSpy).toHaveBeenCalled();
});

it('renders raw metric that is greater than its max value', () => {
it('renders raw metric that is greater than its max value, logs error to console', () => {
const consoleSpy = jest.spyOn(console, 'error').mockImplementation();
const tree = shallow((RunList.prototype as any)._metricCustomRenderer(
{
Expand Down
40 changes: 37 additions & 3 deletions frontend/src/pages/__snapshots__/RunList.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ exports[`RunList displays error in run row if pipeline could not be fetched 1`]
</div>
`;

exports[`RunList loads multiple run 1`] = `
exports[`RunList loads multiple runs 1`] = `
<div>
<CustomTable
columns={
Expand Down Expand Up @@ -6616,6 +6616,40 @@ exports[`RunList reloads the run when refresh is called 1`] = `

exports[`RunList renders a empty metric container when a metric has value of zero 1`] = `<div />`;

exports[`RunList renders a metric container when a percentage metric has value of zero 1`] = `
<div
className="metricContainer"
>
<div
className="metricFill"
style={
Object {
"width": "calc(0.000%)",
}
}
>
0.000%
</div>
</div>
`;

exports[`RunList renders a metric container when a raw metric has value of zero 1`] = `
<div
className="metricContainer"
>
<div
className="metricFill"
style={
Object {
"width": "calc(0%)",
}
}
>
0.000
</div>
</div>
`;

exports[`RunList renders an empty metric when there is no data 1`] = `<div />`;

exports[`RunList renders an empty metric when there is no data 2`] = `<div />`;
Expand Down Expand Up @@ -6713,7 +6747,7 @@ exports[`RunList renders raw metric 1`] = `
</div>
`;

exports[`RunList renders raw metric that is greater than its max value 1`] = `
exports[`RunList renders raw metric that is greater than its max value, logs error to console 1`] = `
<div
style={
Object {
Expand All @@ -6725,7 +6759,7 @@ exports[`RunList renders raw metric that is greater than its max value 1`] = `
</div>
`;

exports[`RunList renders raw metric that is less than its min value 1`] = `
exports[`RunList renders raw metric that is less than its min value, logs error to console 1`] = `
<div
style={
Object {
Expand Down

0 comments on commit e0355de

Please sign in to comment.