From ff5f026539d9c7631ff03b9ea3402bec63b31e9e Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 9 Mar 2022 15:21:00 -0600 Subject: [PATCH] [kbn/optimizer] extract string diffing logic --- packages/kbn-dev-utils/BUILD.bazel | 4 + .../kbn-dev-utils/src/diff_strings.test.ts | 106 ++++++++++++++++++ packages/kbn-dev-utils/src/diff_strings.ts | 97 ++++++++++++++++ packages/kbn-dev-utils/src/index.ts | 1 + packages/kbn-optimizer/BUILD.bazel | 2 - .../src/optimizer/cache_keys.test.ts | 98 +--------------- .../kbn-optimizer/src/optimizer/cache_keys.ts | 77 +------------ 7 files changed, 211 insertions(+), 174 deletions(-) create mode 100644 packages/kbn-dev-utils/src/diff_strings.test.ts create mode 100644 packages/kbn-dev-utils/src/diff_strings.ts diff --git a/packages/kbn-dev-utils/BUILD.bazel b/packages/kbn-dev-utils/BUILD.bazel index e18cdff0ec3ff..5f31a8ba07481 100644 --- a/packages/kbn-dev-utils/BUILD.bazel +++ b/packages/kbn-dev-utils/BUILD.bazel @@ -55,11 +55,13 @@ RUNTIME_DEPS = [ "@npm//exit-hook", "@npm//getopts", "@npm//globby", + "@npm//jest-diff", "@npm//load-json-file", "@npm//markdown-it", "@npm//normalize-path", "@npm//prettier", "@npm//rxjs", + "@npm//strip-ansi", "@npm//sort-package-json", "@npm//tar", "@npm//tree-kill", @@ -90,8 +92,10 @@ TYPES_DEPS = [ "@npm//execa", "@npm//exit-hook", "@npm//getopts", + "@npm//jest-diff", "@npm//rxjs", "@npm//sort-package-json", + "@npm//strip-ansi", "@npm//tree-kill", ] diff --git a/packages/kbn-dev-utils/src/diff_strings.test.ts b/packages/kbn-dev-utils/src/diff_strings.test.ts new file mode 100644 index 0000000000000..411004c8412e6 --- /dev/null +++ b/packages/kbn-dev-utils/src/diff_strings.test.ts @@ -0,0 +1,106 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { diffStrings } from './diff_strings'; + +const json = (x: any) => JSON.stringify(x, null, 2); + +describe('diffStrings()', () => { + it('returns undefined if values are equal', () => { + expect(diffStrings('1', '1')).toBe(undefined); + expect(diffStrings(json(['1', '2', { a: 'b' }]), json(['1', '2', { a: 'b' }]))).toBe(undefined); + expect( + diffStrings( + json({ + a: '1', + b: '2', + }), + json({ + a: '1', + b: '2', + }) + ) + ).toBe(undefined); + }); + + it('returns a diff if the values are different', () => { + const diff = diffStrings(json(['1', '2', { a: 'b' }]), json(['1', '2', { b: 'a' }])); + + expect(diff).toMatchInlineSnapshot(` + "- Expected + + Received + +  [ +  \\"1\\", +  \\"2\\", +  { + - \\"a\\": \\"b\\" + + \\"b\\": \\"a\\" +  } +  ]" + `); + + const diff2 = diffStrings( + json({ + a: '1', + b: '1', + }), + json({ + b: '2', + a: '2', + }) + ); + + expect(diff2).toMatchInlineSnapshot(` + "- Expected + + Received + +  { + - \\"a\\": \\"1\\", + - \\"b\\": \\"1\\" + + \\"b\\": \\"2\\", + + \\"a\\": \\"2\\" +  }" + `); + }); + + it('formats large diffs to focus on the changed lines', () => { + const diff = diffStrings( + json({ + a: ['1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1', '1', '1', '1', '1', '1'], + }), + json({ + b: ['1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1'], + }) + ); + + expect(diff).toMatchInlineSnapshot(` + "- Expected + + Received + +  { + - \\"a\\": [ + + \\"b\\": [ +  \\"1\\", +  \\"1\\", +  ... +  \\"1\\", +  \\"1\\", + - \\"2\\", +  \\"1\\", +  \\"1\\", +  ... +  \\"1\\", +  \\"1\\", + + \\"2\\", +  \\"1\\", +  \\"1\\", +  ..." + `); + }); +}); diff --git a/packages/kbn-dev-utils/src/diff_strings.ts b/packages/kbn-dev-utils/src/diff_strings.ts new file mode 100644 index 0000000000000..11b7e574c7560 --- /dev/null +++ b/packages/kbn-dev-utils/src/diff_strings.ts @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import jestDiff from 'jest-diff'; +import stripAnsi from 'strip-ansi'; +import Chalk from 'chalk'; + +function reformatJestDiff(diff: string) { + const diffLines = diff.split('\n'); + + if ( + diffLines.length < 4 || + stripAnsi(diffLines[0]) !== '- Expected' || + stripAnsi(diffLines[1]) !== '+ Received' + ) { + throw new Error(`unexpected diff format: ${diff}`); + } + + const outputLines = [diffLines.shift(), diffLines.shift(), diffLines.shift()]; + + /** + * buffer which contains between 0 and 5 lines from the diff which aren't additions or + * deletions. The first three are the first three lines seen since the buffer was cleared + * and the last two lines are the last two lines seen. + * + * When flushContext() is called we write the first two lines to output, an elipses if there + * are five lines, and then the last two lines. + * + * At the very end we will write the last two lines of context if they're defined + */ + const contextBuffer: string[] = []; + + /** + * Convert a line to an empty line with elipses placed where the text on that line starts + */ + const toElipses = (line: string) => { + return stripAnsi(line).replace(/^(\s*).*/, '$1...'); + }; + + while (diffLines.length) { + const line = diffLines.shift()!; + const plainLine = stripAnsi(line); + if (plainLine.startsWith('+ ') || plainLine.startsWith('- ')) { + // write contextBuffer to the outputLines + if (contextBuffer.length) { + outputLines.push( + ...contextBuffer.slice(0, 2), + ...(contextBuffer.length === 5 + ? [Chalk.dim(toElipses(contextBuffer[2])), ...contextBuffer.slice(3, 5)] + : contextBuffer.slice(2, 4)) + ); + + contextBuffer.length = 0; + } + + // add this line to the outputLines + outputLines.push(line); + } else { + // update the contextBuffer with this line which doesn't represent a change + if (contextBuffer.length === 5) { + contextBuffer[3] = contextBuffer[4]; + contextBuffer[4] = line; + } else { + contextBuffer.push(line); + } + } + } + + if (contextBuffer.length) { + outputLines.push( + ...contextBuffer.slice(0, 2), + ...(contextBuffer.length > 2 ? [Chalk.dim(toElipses(contextBuffer[2]))] : []) + ); + } + + return outputLines.join('\n'); +} + +/** + * Produces a diff string which is nicely formatted to show the differences between two strings. This will + * be a multi-line string so it's generally a good idea to include a `\n` before this first line of the diff + * if you are concatenating it with another message. + */ +export function diffStrings(expected: string, received: string) { + const diff = jestDiff(expected, received); + + if (!diff || stripAnsi(diff) === 'Compared values have no visual difference.') { + return undefined; + } + + return reformatJestDiff(diff); +} diff --git a/packages/kbn-dev-utils/src/index.ts b/packages/kbn-dev-utils/src/index.ts index fa40a5c37b09b..db96db46bde71 100644 --- a/packages/kbn-dev-utils/src/index.ts +++ b/packages/kbn-dev-utils/src/index.ts @@ -33,3 +33,4 @@ export * from './babel'; export * from './extract'; export * from './vscode_config'; export * from './sort_package_json'; +export * from './diff_strings'; diff --git a/packages/kbn-optimizer/BUILD.bazel b/packages/kbn-optimizer/BUILD.bazel index 680ca0e58b8eb..057e00066ddfa 100644 --- a/packages/kbn-optimizer/BUILD.bazel +++ b/packages/kbn-optimizer/BUILD.bazel @@ -46,7 +46,6 @@ RUNTIME_DEPS = [ "@npm//dedent", "@npm//del", "@npm//execa", - "@npm//jest-diff", "@npm//json-stable-stringify", "@npm//js-yaml", "@npm//lmdb-store", @@ -76,7 +75,6 @@ TYPES_DEPS = [ "@npm//cpy", "@npm//del", "@npm//execa", - "@npm//jest-diff", "@npm//lmdb-store", "@npm//pirates", "@npm//rxjs", diff --git a/packages/kbn-optimizer/src/optimizer/cache_keys.test.ts b/packages/kbn-optimizer/src/optimizer/cache_keys.test.ts index 335a4fd7f74c3..7287362849472 100644 --- a/packages/kbn-optimizer/src/optimizer/cache_keys.test.ts +++ b/packages/kbn-optimizer/src/optimizer/cache_keys.test.ts @@ -8,11 +8,10 @@ import Path from 'path'; -import jestDiff from 'jest-diff'; import { REPO_ROOT } from '@kbn/utils'; import { createAbsolutePathSerializer } from '@kbn/dev-utils'; -import { reformatJestDiff, getOptimizerCacheKey, diffCacheKey } from './cache_keys'; +import { getOptimizerCacheKey } from './cache_keys'; import { OptimizerConfig } from './optimizer_config'; jest.mock('./get_changes.ts', () => ({ @@ -101,98 +100,3 @@ describe('getOptimizerCacheKey()', () => { `); }); }); - -describe('diffCacheKey()', () => { - it('returns undefined if values are equal', () => { - expect(diffCacheKey('1', '1')).toBe(undefined); - expect(diffCacheKey(1, 1)).toBe(undefined); - expect(diffCacheKey(['1', '2', { a: 'b' }], ['1', '2', { a: 'b' }])).toBe(undefined); - expect( - diffCacheKey( - { - a: '1', - b: '2', - }, - { - b: '2', - a: '1', - } - ) - ).toBe(undefined); - }); - - it('returns a diff if the values are different', () => { - expect(diffCacheKey(['1', '2', { a: 'b' }], ['1', '2', { b: 'a' }])).toMatchInlineSnapshot(` - "- Expected - + Received - -  [ -  \\"1\\", -  \\"2\\", -  { - - \\"a\\": \\"b\\" - + \\"b\\": \\"a\\" -  } -  ]" - `); - expect( - diffCacheKey( - { - a: '1', - b: '1', - }, - { - b: '2', - a: '2', - } - ) - ).toMatchInlineSnapshot(` - "- Expected - + Received - -  { - - \\"a\\": \\"1\\", - - \\"b\\": \\"1\\" - + \\"a\\": \\"2\\", - + \\"b\\": \\"2\\" -  }" - `); - }); -}); - -describe('reformatJestDiff()', () => { - it('reformats large jestDiff output to focus on the changed lines', () => { - const diff = jestDiff( - { - a: ['1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1', '1', '1', '1', '1', '1'], - }, - { - b: ['1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '1', '2', '1', '1', '1', '1'], - } - ); - - expect(reformatJestDiff(diff)).toMatchInlineSnapshot(` - "- Expected - + Received - -  Object { - - \\"a\\": Array [ - + \\"b\\": Array [ -  \\"1\\", -  \\"1\\", -  ... -  \\"1\\", -  \\"1\\", - - \\"2\\", -  \\"1\\", -  \\"1\\", -  ... -  \\"1\\", -  \\"1\\", - + \\"2\\", -  \\"1\\", -  \\"1\\", -  ..." - `); - }); -}); diff --git a/packages/kbn-optimizer/src/optimizer/cache_keys.ts b/packages/kbn-optimizer/src/optimizer/cache_keys.ts index da06b76327a8b..a30dbe27dff08 100644 --- a/packages/kbn-optimizer/src/optimizer/cache_keys.ts +++ b/packages/kbn-optimizer/src/optimizer/cache_keys.ts @@ -9,12 +9,10 @@ import Path from 'path'; import Fs from 'fs'; -import Chalk from 'chalk'; import execa from 'execa'; import { REPO_ROOT } from '@kbn/utils'; -import stripAnsi from 'strip-ansi'; +import { diffStrings } from '@kbn/dev-utils'; -import jestDiff from 'jest-diff'; import jsonStable from 'json-stable-stringify'; import { ascending, CacheableWorkerConfig } from '../common'; @@ -36,78 +34,7 @@ export function diffCacheKey(expected?: unknown, actual?: unknown) { return; } - return reformatJestDiff(jestDiff(expectedJson, actualJson)); -} - -export function reformatJestDiff(diff: string | null) { - const diffLines = diff?.split('\n') || []; - - if ( - diffLines.length < 4 || - stripAnsi(diffLines[0]) !== '- Expected' || - stripAnsi(diffLines[1]) !== '+ Received' - ) { - throw new Error(`unexpected diff format: ${diff}`); - } - - const outputLines = [diffLines.shift(), diffLines.shift(), diffLines.shift()]; - - /** - * buffer which contains between 0 and 5 lines from the diff which aren't additions or - * deletions. The first three are the first three lines seen since the buffer was cleared - * and the last two lines are the last two lines seen. - * - * When flushContext() is called we write the first two lines to output, an elipses if there - * are five lines, and then the last two lines. - * - * At the very end we will write the last two lines of context if they're defined - */ - const contextBuffer: string[] = []; - - /** - * Convert a line to an empty line with elipses placed where the text on that line starts - */ - const toElipses = (line: string) => { - return stripAnsi(line).replace(/^(\s*).*/, '$1...'); - }; - - while (diffLines.length) { - const line = diffLines.shift()!; - const plainLine = stripAnsi(line); - if (plainLine.startsWith('+ ') || plainLine.startsWith('- ')) { - // write contextBuffer to the outputLines - if (contextBuffer.length) { - outputLines.push( - ...contextBuffer.slice(0, 2), - ...(contextBuffer.length === 5 - ? [Chalk.dim(toElipses(contextBuffer[2])), ...contextBuffer.slice(3, 5)] - : contextBuffer.slice(2, 4)) - ); - - contextBuffer.length = 0; - } - - // add this line to the outputLines - outputLines.push(line); - } else { - // update the contextBuffer with this line which doesn't represent a change - if (contextBuffer.length === 5) { - contextBuffer[3] = contextBuffer[4]; - contextBuffer[4] = line; - } else { - contextBuffer.push(line); - } - } - } - - if (contextBuffer.length) { - outputLines.push( - ...contextBuffer.slice(0, 2), - ...(contextBuffer.length > 2 ? [Chalk.dim(toElipses(contextBuffer[2]))] : []) - ); - } - - return outputLines.join('\n'); + return diffStrings(expectedJson, actualJson); } export interface OptimizerCacheKey {