Skip to content

Commit

Permalink
[UI] deep links to pipeline details page from start page (#3086)
Browse files Browse the repository at this point in the history
* [UI] deep links to pipeline details page from start page

* Fix

* Update GettingStarted.tsx

* Update GettingStarted.tsx

* Update GettingStarted.tsx

* Adjust format to improve readability

* Use react-testing-library for getting started page tests

* Add error case unit tests

* Frontend import sample config from jsonn and presubmit test to verify configs are synced

* Update presubmit test error message

* Changed to sync only sample_config.json name to frontend

* Improve error message

* Fix tests
  • Loading branch information
Bobgy authored Feb 17, 2020
1 parent d0ef0ae commit 9b723bf
Show file tree
Hide file tree
Showing 12 changed files with 711 additions and 17 deletions.
162 changes: 162 additions & 0 deletions frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"start:proxy-standalone": "./start-proxy-standalone.sh",
"start:proxy-standalone-and-server": "./start-proxy-standalone-and-server.sh",
"start": "react-scripts-ts start",
"sync-backend-sample-config": "node scripts/sync-backend-sample-config.js",
"test": "react-scripts-ts test --env=jsdom",
"test:server:coverage": "cd ./server && npm test -- --coverage && cd ..",
"test:coverage": "npm test -- --coverage && npm run test:server:coverage",
Expand All @@ -61,6 +62,7 @@
},
"devDependencies": {
"@google-cloud/storage": "^4.1.3",
"@testing-library/react": "^9.4.0",
"@types/d3": "^5.0.0",
"@types/d3-dsv": "^1.0.33",
"@types/dagre": "^0.7.40",
Expand All @@ -74,6 +76,7 @@
"@types/lodash": ">=4.14.117",
"@types/markdown-to-jsx": "^6.9.0",
"@types/node": "^10.17.11",
"@types/prettier": "^1.19.0",
"@types/react": "^16.7.18",
"@types/react-dom": "^16.0.7",
"@types/react-router-dom": "^4.3.1",
Expand Down
11 changes: 11 additions & 0 deletions frontend/scripts/sync-backend-sample-config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// This should be run in pipelines/frontend folder.

const PATH_BACKEND_CONFIG = '../../backend/src/apiserver/config/sample_config.json';
const PATH_FRONTEND_CONFIG = 'src/config/sample_config_from_backend.json';

const fs = require('fs');
const backendConfig = require(PATH_BACKEND_CONFIG);
const frontendConfig = backendConfig.map(sample => sample.name);
const content = JSON.stringify(frontendConfig, null, 2) + '\n';
fs.writeFileSync(PATH_FRONTEND_CONFIG, content);
console.log(`Synced ${PATH_BACKEND_CONFIG} to ${PATH_FRONTEND_CONFIG}`);
5 changes: 5 additions & 0 deletions frontend/src/TestUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ToolbarActionConfig } from './components/Toolbar';
import { match } from 'react-router';
import { mount, ReactWrapper } from 'enzyme';
import { object } from 'prop-types';
import { format } from 'prettier';

export default class TestUtils {
/**
Expand Down Expand Up @@ -102,3 +103,7 @@ export default class TestUtils {
return lastCall.actions[buttonKey];
}
}

export function formatHTML(html: string): string {
return format(html, { parser: 'html' });
}
9 changes: 9 additions & 0 deletions frontend/src/atoms/ExternalLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,12 @@ const css = stylesheet({
export const ExternalLink: React.FC<
DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>
> = props => <a {...props} className={css.link} target='_blank' rel='noopener' />;

export const AutoLink: React.FC<
DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>
> = props =>
props.href && props.href.startsWith('#') ? (
<a {...props} className={css.link} />
) : (
<ExternalLink {...props} />
);
3 changes: 3 additions & 0 deletions frontend/src/components/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export const RoutePageFactory = {
artifactType,
).replace(`:${RouteParams.ID}`, '' + artifactId);
},
pipelineDetails: (id: string) => {
return RoutePage.PIPELINE_DETAILS_NO_VERSION.replace(`:${RouteParams.pipelineId}`, id);
},
};

export const ExternalLinks = {
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/config/sample_config_from_backend.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
"[Demo] XGBoost - Training with Confusion Matrix",
"[Demo] TFX - Taxi Tip Prediction Model Trainer",
"[Tutorial] Data passing in python components",
"[Tutorial] DSL - Control structures"
]
98 changes: 98 additions & 0 deletions frontend/src/pages/GettingStarted.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import React from 'react';
import { GettingStarted } from './GettingStarted';
import TestUtils, { formatHTML } from '../TestUtils';
import { render } from '@testing-library/react';
import { PageProps } from './Page';
import { Apis } from '../lib/Apis';
import { ApiListPipelinesResponse } from 'src/apis/pipeline/api';
import snapshotDiff from 'snapshot-diff';

const PATH_BACKEND_CONFIG = '../../../backend/src/apiserver/config/sample_config.json';
const PATH_FRONTEND_CONFIG = '../config/sample_config_from_backend.json';
describe(`${PATH_FRONTEND_CONFIG}`, () => {
it(`should be in sync with ${PATH_BACKEND_CONFIG}, if not please run "npm run sync-backend-sample-config" to update.`, () => {
const configBackend = require(PATH_BACKEND_CONFIG);
const configFrontend = require(PATH_FRONTEND_CONFIG);
expect(configFrontend).toEqual(configBackend.map((sample: any) => sample.name));
});
});

describe('GettingStarted page', () => {
const updateBannerSpy = jest.fn();
const updateToolbarSpy = jest.fn();
const historyPushSpy = jest.fn();
const pipelineListSpy = jest.spyOn(Apis.pipelineServiceApi, 'listPipelines');

function generateProps(): PageProps {
return TestUtils.generatePageProps(
GettingStarted,
{} as any,
{} as any,
historyPushSpy,
updateBannerSpy,
null,
updateToolbarSpy,
null,
);
}

beforeEach(() => {
jest.resetAllMocks();
const empty: ApiListPipelinesResponse = {
pipelines: [],
total_size: 0,
};
pipelineListSpy.mockImplementation(() => Promise.resolve(empty));
});

it('initially renders documentation', () => {
const { container } = render(<GettingStarted {...generateProps()} />);
expect(container).toMatchSnapshot();
});

it('renders documentation with pipeline deep link after querying demo pipelines', async () => {
let count = 0;
pipelineListSpy.mockImplementation(() => {
++count;
const response: ApiListPipelinesResponse = {
pipelines: [{ id: `pipeline-id-${count}` }],
};
return Promise.resolve(response);
});
const { container } = render(<GettingStarted {...generateProps()} />);
const initialHtml = container.innerHTML;
await TestUtils.flushPromises();
expect(pipelineListSpy.mock.calls).toMatchSnapshot();
expect(
snapshotDiff(formatHTML(initialHtml), formatHTML(container.innerHTML)),
).toMatchSnapshot();
});

it('fallbacks to show pipeline list page if request failed', async () => {
let count = 0;
pipelineListSpy.mockImplementation(
(): Promise<ApiListPipelinesResponse> => {
++count;
if (count === 1) {
return Promise.reject(new Error('Mocked error'));
} else if (count === 2) {
// incomplete data
return Promise.resolve({});
} else if (count === 3) {
// empty data
return Promise.resolve({ pipelines: [], total_size: 0 });
}
return Promise.resolve({
pipelines: [{ id: `pipeline-id-${count}` }],
total_size: 1,
});
},
);
const { container } = render(<GettingStarted {...generateProps()} />);
const initialHtml = container.innerHTML;
await TestUtils.flushPromises();
expect(
snapshotDiff(formatHTML(initialHtml), formatHTML(container.innerHTML)),
).toMatchSnapshot();
});
});
Loading

0 comments on commit 9b723bf

Please sign in to comment.