From cf531a08b1a73ea89403ee762e4315b7bb5b3eeb Mon Sep 17 00:00:00 2001 From: "Yuan (Bob) Gong" Date: Tue, 25 Feb 2020 14:38:48 +0800 Subject: [PATCH] [Frontend] snapshot diff setup (#3166) * [Frontend] snapshot diff setup * Rename * Fix test * Update README about recommended setup --- frontend/README.md | 9 +- frontend/package.json | 1 + frontend/src/TestUtils.tsx | 42 ++++++++ frontend/src/components/SideNav.test.tsx | 33 +++++- .../__snapshots__/SideNav.test.tsx.snap | 28 ----- .../components/viewers/Tensorboard.test.tsx | 63 +++++++++-- .../__snapshots__/Tensorboard.test.tsx.snap | 54 ---------- frontend/src/pages/GettingStarted.test.tsx | 100 ++++++++++++++++-- .../GettingStarted.test.tsx.snap | 89 ---------------- 9 files changed, 223 insertions(+), 196 deletions(-) diff --git a/frontend/README.md b/frontend/README.md index 95166bd933c7..f0ff21c9dd9d 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -100,13 +100,16 @@ To understand more what prettier is: [What is Prettier](https://prettier.io/docs this project's config automatically. Recommend setting the following in [settings.json](https://code.visualstudio.com/docs/getstarted/settings#_settings-file-locations) for vscode to autoformat on save. ``` - "[typescript]": { - "editor.formatOnSave": true, + "[typescript]": { + "editor.formatOnSave": true, + "files.trimTrailingWhitespace": false, }, "[typescriptreact]": { - "editor.formatOnSave": true, + "editor.formatOnSave": true, + "files.trimTrailingWhitespace": false, }, ``` + Also, vscode builtin trailing whitespace [conflicts with jest inline snapshot](https://github.com/Microsoft/vscode/issues/52711), so recommend disabling it. - For others, refer to https://prettier.io/docs/en/editors.html ### Format Code Manually diff --git a/frontend/package.json b/frontend/package.json index 0ad9c78bfd2c..892bac924473 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -110,6 +110,7 @@ ], "snapshotSerializers": [ "./src/__serializers__/mock-function", + "snapshot-diff/serializer.js", "enzyme-to-json/serializer" ] }, diff --git a/frontend/src/TestUtils.tsx b/frontend/src/TestUtils.tsx index 3f1646d40ee1..f82e2219bf7b 100644 --- a/frontend/src/TestUtils.tsx +++ b/frontend/src/TestUtils.tsx @@ -26,6 +26,7 @@ import { match } from 'react-router'; import { mount, ReactWrapper } from 'enzyme'; import { object } from 'prop-types'; import { format } from 'prettier'; +import snapshotDiff from 'snapshot-diff'; export default class TestUtils { /** @@ -110,3 +111,44 @@ export default class TestUtils { export function formatHTML(html: string): string { return format(html, { parser: 'html' }); } + +/** + * Generate diff text for two HTML strings. + * Recommend providing base and update annotations to clarify context in the diff directly. + */ +export function diffHTML({ + base, + update, + baseAnnotation, + updateAnnotation, +}: { + base: string; + baseAnnotation?: string; + update: string; + updateAnnotation?: string; +}) { + return diff({ + base: formatHTML(base), + update: formatHTML(update), + baseAnnotation, + updateAnnotation, + }); +} + +export function diff({ + base, + update, + baseAnnotation, + updateAnnotation, +}: { + base: string; + baseAnnotation?: string; + update: string; + updateAnnotation?: string; +}) { + return snapshotDiff(base, update, { + stablePatchmarks: true, // Avoid line numbers in diff, so that diffs are stable against irrelevant changes + aAnnotation: baseAnnotation, + bAnnotation: updateAnnotation, + }); +} diff --git a/frontend/src/components/SideNav.test.tsx b/frontend/src/components/SideNav.test.tsx index ca442a31fb5f..b04ddc210e27 100644 --- a/frontend/src/components/SideNav.test.tsx +++ b/frontend/src/components/SideNav.test.tsx @@ -17,13 +17,12 @@ import * as React from 'react'; import SideNav, { css } from './SideNav'; -import TestUtils from '../TestUtils'; +import TestUtils, { diff } from '../TestUtils'; import { Apis } from '../lib/Apis'; import { LocalStorage } from '../lib/LocalStorage'; import { ReactWrapper, ShallowWrapper, shallow } from 'enzyme'; import { RoutePage } from './Router'; import { RouterProps } from 'react-router'; -import snapshotDiff from 'snapshot-diff'; const wideWidth = 1000; const narrowWidth = 200; @@ -259,10 +258,36 @@ describe('SideNav', () => { buildInfoSpy.mockImplementationOnce(() => Promise.reject('Error when fetching build info')); tree = shallow(); - const baseTree = tree.debug(); + const base = tree.debug(); await TestUtils.flushPromises(); tree.update(); - expect(snapshotDiff(baseTree, tree.debug())).toMatchSnapshot(); + expect(diff({ base, update: tree.debug() })).toMatchInlineSnapshot(` + Snapshot Diff: + - Expected + + Received + + @@ --- --- @@ + + + + +
+ + + +
+ + + + Cluster name: + + + + + + some-cluster-name + + + +
+ + + +
+ + Report an Issue + + `); }); it('displays the frontend commit hash if the api server hash is not returned', async () => { diff --git a/frontend/src/components/__snapshots__/SideNav.test.tsx.snap b/frontend/src/components/__snapshots__/SideNav.test.tsx.snap index cd6f1565020b..7a073125cc14 100644 --- a/frontend/src/components/__snapshots__/SideNav.test.tsx.snap +++ b/frontend/src/components/__snapshots__/SideNav.test.tsx.snap @@ -1,33 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`SideNav populates the cluster information using the response from the system endpoints 1`] = ` -"Snapshot Diff: -- First value -+ Second value - -@@ -64,10 +64,20 @@ - - - -
-
-+ -+
-+ -+ Cluster name: -+ -+ -+ some-cluster-name -+ -+
-+ - -
- - Report an Issue - " -`; - exports[`SideNav populates the display build information using the response from the healthz endpoint 1`] = `
{ let tree: ReactWrapper | ShallowWrapper; @@ -50,10 +49,26 @@ describe('Tensorboard', () => { const getAppMock = () => Promise.resolve({ podAddress: '', tfVersion: '' }); jest.spyOn(Apis, 'getTensorboardApp').mockImplementation(getAppMock); tree = shallow(); - const baseTree = tree.debug(); + const base = tree.debug(); await TestUtils.flushPromises(); - expect(snapshotDiff(tree.debug(), baseTree)).toMatchSnapshot(); + expect(diff({ base, update: tree.debug() })).toMatchInlineSnapshot(` + Snapshot Diff: + - Expected + + Received + + @@ --- --- @@ + + + +
+
+ - + + +
+
+
+ `); }); it('does not break on empty data', async () => { @@ -61,10 +76,26 @@ describe('Tensorboard', () => { jest.spyOn(Apis, 'getTensorboardApp').mockImplementation(getAppMock); const config = { type: PlotType.TENSORBOARD, url: '' }; tree = shallow(); - const baseTree = tree.debug(); + const base = tree.debug(); await TestUtils.flushPromises(); - expect(snapshotDiff(tree.debug(), baseTree)).toMatchSnapshot(); + expect(diff({ base, update: tree.debug() })).toMatchInlineSnapshot(` + Snapshot Diff: + - Expected + + Received + + @@ --- --- @@ + + + +
+
+ - + + +
+ + + `); }); it('shows a link to the tensorboard instance if exists', async () => { @@ -82,10 +113,26 @@ describe('Tensorboard', () => { const getAppMock = () => Promise.resolve({ podAddress: '', tfVersion: '' }); const spy = jest.spyOn(Apis, 'getTensorboardApp').mockImplementation(getAppMock); tree = shallow(); - const baseTree = tree.debug(); + const base = tree.debug(); await TestUtils.flushPromises(); - expect(snapshotDiff(tree.debug(), baseTree)).toMatchSnapshot(); + expect(diff({ base, update: tree.debug() })).toMatchInlineSnapshot(` + Snapshot Diff: + - Expected + + Received + + @@ --- --- @@ + + + + +
+ - + + +
+ + + `); expect(spy).toHaveBeenCalledWith(config.url); }); diff --git a/frontend/src/components/viewers/__snapshots__/Tensorboard.test.tsx.snap b/frontend/src/components/viewers/__snapshots__/Tensorboard.test.tsx.snap index ce2c0b3ce0ad..ff0e334cc18d 100644 --- a/frontend/src/components/viewers/__snapshots__/Tensorboard.test.tsx.snap +++ b/frontend/src/components/viewers/__snapshots__/Tensorboard.test.tsx.snap @@ -111,42 +111,6 @@ exports[`Tensorboard base component snapshot 1`] = ` `; -exports[`Tensorboard does not break on empty data 1`] = ` -"Snapshot Diff: -- First value -+ Second value - -@@ -53,9 +53,9 @@ - - - - -
-- -+ -
- - " -`; - -exports[`Tensorboard does not break on no config 1`] = ` -"Snapshot Diff: -- First value -+ Second value - -@@ -53,9 +53,9 @@ - - - - -
-- -+ -
- - " -`; - exports[`Tensorboard shows a link to the tensorboard instance if exists 1`] = `
@@ -218,21 +182,3 @@ exports[`Tensorboard shows a link to the tensorboard instance if exists 1`] = `
`; - -exports[`Tensorboard shows start button if no instance exists 1`] = ` -"Snapshot Diff: -- First value -+ Second value - -@@ -53,9 +53,9 @@ - - - - -
-- -+ -
- - " -`; diff --git a/frontend/src/pages/GettingStarted.test.tsx b/frontend/src/pages/GettingStarted.test.tsx index 5d8c624519ae..c1820be517ad 100644 --- a/frontend/src/pages/GettingStarted.test.tsx +++ b/frontend/src/pages/GettingStarted.test.tsx @@ -1,11 +1,10 @@ import React from 'react'; import { GettingStarted } from './GettingStarted'; -import TestUtils, { formatHTML } from '../TestUtils'; +import TestUtils, { diffHTML } from '../TestUtils'; import { render } from '@testing-library/react'; import { PageProps } from './Page'; import { Apis } from '../lib/Apis'; import { ApiListPipelinesResponse } from '../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'; @@ -60,12 +59,75 @@ describe('GettingStarted page', () => { return Promise.resolve(response); }); const { container } = render(); - const initialHtml = container.innerHTML; + const base = container.innerHTML; await TestUtils.flushPromises(); expect(pipelineListSpy.mock.calls).toMatchSnapshot(); - expect( - snapshotDiff(formatHTML(initialHtml), formatHTML(container.innerHTML)), - ).toMatchSnapshot(); + expect(diffHTML({ base, update: container.innerHTML })).toMatchInlineSnapshot(` + Snapshot Diff: + - Expected + + Received + + @@ --- --- @@ +

Demonstrations and Tutorials

+

This section contains demo and tutorial pipelines.

+

Demos - Try an end-to-end demonstration pipeline.

+