Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(frontend): Create sections and run list for KFPv2 Run Comparison page #7882

Merged
merged 13 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions frontend/src/components/CollapseButtonSingle.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2022 The Kubeflow Authors
*
* 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
*
* https://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 * as React from 'react';
import { render, screen, fireEvent } from '@testing-library/react';
import CollapseButtonSingle from './CollapseButtonSingle';

describe('CollapseButtonSingle', () => {
const collapseSectionUpdateSpy = jest.fn();

it('Collapes an expanded section when clicked', () => {
render(
<CollapseButtonSingle
collapseSection={false}
collapseSectionUpdate={collapseSectionUpdateSpy}
sectionName='testSection'
/>,
);

fireEvent.click(screen.getByText('testSection'));
expect(collapseSectionUpdateSpy).toHaveBeenCalledWith(true);
});

it('Expands a collapsed section when clicked', () => {
render(
<CollapseButtonSingle
collapseSection={true}
collapseSectionUpdate={collapseSectionUpdateSpy}
sectionName='testSection'
/>,
);

fireEvent.click(screen.getByText('testSection'));
expect(collapseSectionUpdateSpy).toHaveBeenCalledWith(false);
});
});
64 changes: 64 additions & 0 deletions frontend/src/components/CollapseButtonSingle.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright 2022 The Kubeflow Authors
*
* 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
*
* https://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 * as React from 'react';
import Button from '@material-ui/core/Button';
import ExpandedIcon from '@material-ui/icons/ArrowDropUp';
import { stylesheet, classes } from 'typestyle';
import { color, fontsize } from '../Css';

const css = stylesheet({
collapseBtn: {
color: color.strong,
fontSize: fontsize.title,
fontWeight: 'bold',
padding: 5,
},
collapsed: {
transform: 'rotate(180deg)',
},
icon: {
marginRight: 5,
transition: 'transform 0.3s',
},
});

interface CollapseButtonSingleProps {
collapseSection: boolean;
collapseSectionUpdate: (collapseSection: boolean) => void;
sectionName: string;
}

function CollapseButtonSingle(props: CollapseButtonSingleProps) {
const { collapseSection, collapseSectionUpdate, sectionName } = props;

return (
<div>
<Button
onClick={() => {
collapseSectionUpdate(!collapseSection);
}}
title='Expand/Collapse this section'
className={css.collapseBtn}
>
<ExpandedIcon className={classes(css.icon, collapseSection ? css.collapsed : '')} />
{sectionName}
</Button>
</div>
);
}

export default CollapseButtonSingle;
22 changes: 10 additions & 12 deletions frontend/src/pages/Compare.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);
await TestUtils.flushPromises();

await waitFor(() => screen.getByText('Run overview'));
await waitFor(() => expect(screen.queryByText('Parameter Section V2')).toBeNull());
});

it('Show mixed version runs page error if run versions are mixed between v1 and v2', async () => {
Expand All @@ -140,7 +141,6 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);

await TestUtils.flushPromises();

await waitFor(() =>
Expand Down Expand Up @@ -175,7 +175,6 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...props} />
</CommonTestWrapper>,
);

await TestUtils.flushPromises();

await waitFor(() =>
Expand Down Expand Up @@ -221,7 +220,6 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...props} />
</CommonTestWrapper>,
);

await TestUtils.flushPromises();

await waitFor(() =>
Expand Down Expand Up @@ -252,10 +250,9 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);

await TestUtils.flushPromises();

await waitFor(() => screen.getByText('Run overview'));
await waitFor(() => expect(screen.queryByText('Parameter Section V2')).toBeNull());
});

it('Show no error on v1 page if there are less than two runs and v2 feature flag disabled', async () => {
Expand All @@ -273,10 +270,9 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...props} />
</CommonTestWrapper>,
);

await TestUtils.flushPromises();

await waitFor(() => screen.getByText('Run overview'));
await waitFor(() => expect(screen.queryByText('Parameter Section V2')).toBeNull());
});

it('Show v2 page if all runs are v2 and the v2 feature flag is enabled', async () => {
Expand All @@ -301,8 +297,9 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);
await TestUtils.flushPromises();

await waitFor(() => screen.getByText('This is the V2 Run Comparison page.'));
await waitFor(() => screen.getByText('Parameter Section V2'));
});

it('Show v1 page if some runs are v1 and the v2 feature flag is disabled', async () => {
Expand All @@ -322,8 +319,9 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);
await TestUtils.flushPromises();

await waitFor(() => screen.getByText('Run overview'));
await waitFor(() => expect(screen.queryByText('Parameter Section V2')).toBeNull());
});

it('Show v1 page if all runs are v2 and the v2 feature flag is disabled', async () => {
Expand All @@ -343,8 +341,9 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);
await TestUtils.flushPromises();

await waitFor(() => screen.getByText('Run overview'));
await waitFor(() => expect(screen.queryByText('Parameter Section V2')).toBeNull());
});

it('Show page error on page when getRun request fails', async () => {
Expand Down Expand Up @@ -373,7 +372,6 @@ describe('Switch between v1 and v2 Run Comparison pages', () => {
<Compare {...generateProps()} />
</CommonTestWrapper>,
);

await TestUtils.flushPromises();

await waitFor(() =>
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/pages/Compare.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ enum CompareVersion {
Unknown,
}

export const OVERVIEW_SECTION_NAME = 'Run overview';
export const PARAMS_SECTION_NAME = 'Parameters';
export const METRICS_SECTION_NAME = 'Metrics';

// This is a router to determine whether to show V1 or V2 compare page.
export default function Compare(props: PageProps) {
const { updateBanner } = props;
Expand Down Expand Up @@ -109,7 +113,7 @@ export default function Compare(props: PageProps) {
// Clear the banner unless the V1 page is shown, as that page handles its own banner state.
updateBanner({});
}
}, [compareVersion, isError, error, updateBanner, runIds.length]);
}, [compareVersion, isError, error, isLoading, updateBanner, runIds.length]);
zpChris marked this conversation as resolved.
Show resolved Hide resolved

if (isError || isLoading) {
return <></>;
Expand All @@ -120,7 +124,7 @@ export default function Compare(props: PageProps) {
}

if (compareVersion === CompareVersion.V2) {
return <CompareV2 />;
return <CompareV2 {...props} />;
}

return <></>;
Expand Down
27 changes: 14 additions & 13 deletions frontend/src/pages/CompareV1.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { ButtonKeys } from '../lib/Buttons';
import { render } from '@testing-library/react';
import { Router } from 'react-router-dom';
import { NamespaceContext } from 'src/lib/KubeflowClient';
import { METRICS_SECTION_NAME, OVERVIEW_SECTION_NAME, PARAMS_SECTION_NAME } from './Compare';

const CompareV1 = TEST_ONLY.CompareV1;
class TestCompare extends CompareV1 {
Expand Down Expand Up @@ -443,17 +444,17 @@ describe('CompareV1', () => {

it('collapses all sections', async () => {
await setUpViewersAndShallowMount();
const instance = tree.instance() as Compare;
const instance = tree.instance() as CompareV1;
const collapseBtn = instance.getInitialToolbarState().actions[ButtonKeys.COLLAPSE];

expect(tree.state('collapseSections')).toEqual({});

collapseBtn!.action();

expect(tree.state('collapseSections')).toEqual({
Metrics: true,
Parameters: true,
'Run overview': true,
[METRICS_SECTION_NAME]: true,
[PARAMS_SECTION_NAME]: true,
[OVERVIEW_SECTION_NAME]: true,
Table: true,
Tensorboard: true,
});
Expand All @@ -463,7 +464,7 @@ describe('CompareV1', () => {

it('expands all sections if they were collapsed', async () => {
await setUpViewersAndShallowMount();
const instance = tree.instance() as Compare;
const instance = tree.instance() as CompareV1;
const collapseBtn = instance.getInitialToolbarState().actions[ButtonKeys.COLLAPSE];
const expandBtn = instance.getInitialToolbarState().actions[ButtonKeys.EXPAND];

Expand All @@ -472,9 +473,9 @@ describe('CompareV1', () => {
collapseBtn!.action();

expect(tree.state('collapseSections')).toEqual({
Metrics: true,
Parameters: true,
'Run overview': true,
[METRICS_SECTION_NAME]: true,
[PARAMS_SECTION_NAME]: true,
[OVERVIEW_SECTION_NAME]: true,
Table: true,
Tensorboard: true,
});
Expand All @@ -499,7 +500,7 @@ describe('CompareV1', () => {
.find('button')
.simulate('click');

expect(tree.state('collapseSections')).toEqual({ 'Run overview': true });
expect(tree.state('collapseSections')).toEqual({ [OVERVIEW_SECTION_NAME]: true });

// Collapse run parameters
tree
Expand All @@ -509,8 +510,8 @@ describe('CompareV1', () => {
.simulate('click');

expect(tree.state('collapseSections')).toEqual({
Parameters: true,
'Run overview': true,
[PARAMS_SECTION_NAME]: true,
[OVERVIEW_SECTION_NAME]: true,
});

// Re-expand run overview and parameters
Expand All @@ -526,8 +527,8 @@ describe('CompareV1', () => {
.simulate('click');

expect(tree.state('collapseSections')).toEqual({
Parameters: false,
'Run overview': false,
[PARAMS_SECTION_NAME]: false,
[OVERVIEW_SECTION_NAME]: false,
});
});

Expand Down
Loading