From 6e07b300487d69374eb1f31599ff2bda65fbac05 Mon Sep 17 00:00:00 2001 From: Connor Lindsey Date: Thu, 17 Feb 2022 08:34:16 -0700 Subject: [PATCH] Chore: Clean up logs and deprecated library in tracing tests (#45486) * Fix logs and errors from tracing tests * Refactor SpanBar to remove recompose * Fix NodeGraph tests --- .betterer.results | 2 +- packages/jaeger-ui-components/package.json | 2 - .../src/TraceTimelineViewer/SpanBar.test.js | 9 +- .../src/TraceTimelineViewer/SpanBar.tsx | 69 ++++------ packages/jaeger-ui-components/src/index.ts | 6 - .../datasource/tempo/datasource.test.ts | 6 + .../datasource/zipkin/QueryField.test.tsx | 7 +- .../panel/nodeGraph/NodeGraph.test.tsx | 18 ++- yarn.lock | 118 +----------------- 9 files changed, 61 insertions(+), 176 deletions(-) diff --git a/.betterer.results b/.betterer.results index 3e95eafec50..3a1f939d9d5 100644 --- a/.betterer.results +++ b/.betterer.results @@ -110,7 +110,7 @@ exports[`no enzyme tests`] = { "packages/jaeger-ui-components/src/TraceTimelineViewer/ReferencesButton.test.js:2288177454": [ [15, 19, 13, "RegExp match", "2409514259"] ], - "packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js:2545268168": [ + "packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js:2127169675": [ [15, 17, 13, "RegExp match", "2409514259"] ], "packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBarRow.test.js:2454947085": [ diff --git a/packages/jaeger-ui-components/package.json b/packages/jaeger-ui-components/package.json index ce1308cecb6..c68c7169f87 100644 --- a/packages/jaeger-ui-components/package.json +++ b/packages/jaeger-ui-components/package.json @@ -18,7 +18,6 @@ "@types/prop-types": "^15", "@types/react": "17.0.39", "@types/react-icons": "2.2.7", - "@types/recompose": "0.30.10", "@types/reselect": "2.2.0", "@types/tinycolor2": "1.4.3", "enzyme": "3.11.0", @@ -46,7 +45,6 @@ "react": "17.0.2", "react-dom": "17.0.2", "react-icons": "2.2.7", - "recompose": "0.30.0", "reselect": "4.1.5", "tinycolor2": "1.4.2", "tslib": "2.3.1", diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js index 09146b8e459..70cc7b87420 100644 --- a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js +++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.test.js @@ -14,6 +14,7 @@ import React from 'react'; import { mount } from 'enzyme'; +import { act } from 'react-dom/test-utils'; import { Popover } from '../common/Popover'; import SpanBar from './SpanBar'; @@ -79,9 +80,13 @@ describe('', () => { const { onMouseOver, onMouseLeave } = wrapper.find('[data-test-id="SpanBar--wrapper"]').props(); const labelElm = wrapper.find('[data-test-id="SpanBar--label"]'); expect(labelElm.text()).toBe(shortLabel); - onMouseOver(); + act(() => { + onMouseOver(); + }); expect(labelElm.text()).toBe(longLabel); - onMouseLeave(); + act(() => { + onMouseLeave(); + }); expect(labelElm.text()).toBe(shortLabel); }); diff --git a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.tsx b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.tsx index c78db4b23ae..097bd362bda 100644 --- a/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.tsx +++ b/packages/jaeger-ui-components/src/TraceTimelineViewer/SpanBar.tsx @@ -15,8 +15,7 @@ import cx from 'classnames'; import { css } from '@emotion/css'; import { groupBy as _groupBy } from 'lodash'; -import React from 'react'; -import { compose, onlyUpdateForKeys, withProps, withState } from 'recompose'; +import React, { useState } from 'react'; import { GrafanaTheme2 } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; import { autoColor } from '../Theme'; @@ -90,7 +89,7 @@ const getStyles = (theme: GrafanaTheme2) => { }; }; -type TCommonProps = { +type Props = { color: string; onClick?: (evt: React.MouseEvent) => void; viewEnd: number; @@ -107,39 +106,32 @@ type TCommonProps = { span: TraceSpan; className?: string; labelClassName?: string; -}; - -type TInnerProps = { - label: string; - setLongLabel: () => void; - setShortLabel: () => void; -} & TCommonProps; - -type TOuterProps = { longLabel: string; shortLabel: string; -} & TCommonProps; +}; function toPercent(value: number) { return `${(value * 100).toFixed(1)}%`; } -function SpanBar(props: TInnerProps) { - const { - viewEnd, - viewStart, - getViewedBounds, - color, - label, - onClick, - setLongLabel, - setShortLabel, - rpc, - traceStartTime, - span, - className, - labelClassName, - } = props; +function SpanBar({ + viewEnd, + viewStart, + getViewedBounds, + color, + shortLabel, + longLabel, + onClick, + rpc, + traceStartTime, + span, + className, + labelClassName, +}: Props) { + const [label, setLabel] = useState(shortLabel); + const setShortLabel = () => setLabel(shortLabel); + const setLongLabel = () => setLabel(longLabel); + // group logs based on timestamps const logGroups = _groupBy(span.logs, (log) => { const posPercent = getViewedBounds(log.timestamp, log.timestamp).start; @@ -196,21 +188,4 @@ function SpanBar(props: TInnerProps) { ); } -export default compose( - withState('label', 'setLabel', (props: { shortLabel: string }) => props.shortLabel), - withProps( - ({ - setLabel, - shortLabel, - longLabel, - }: { - setLabel: (label: string) => void; - shortLabel: string; - longLabel: string; - }) => ({ - setLongLabel: () => setLabel(longLabel), - setShortLabel: () => setLabel(shortLabel), - }) - ), - onlyUpdateForKeys(['label', 'rpc', 'viewStart', 'viewEnd']) -)(SpanBar); +export default React.memo(SpanBar); diff --git a/packages/jaeger-ui-components/src/index.ts b/packages/jaeger-ui-components/src/index.ts index 64ad6a2b383..f5737eff1f5 100644 --- a/packages/jaeger-ui-components/src/index.ts +++ b/packages/jaeger-ui-components/src/index.ts @@ -6,9 +6,3 @@ export { default as DetailState } from './TraceTimelineViewer/SpanDetail/DetailS export { default as transformTraceData } from './model/transform-trace-data'; export { default as filterSpans } from './utils/filter-spans'; export * from './Theme'; - -import { onlyUpdateForKeys } from 'recompose'; - -export default { - onlyUpdateForKeys, -} as any; diff --git a/public/app/plugins/datasource/tempo/datasource.test.ts b/public/app/plugins/datasource/tempo/datasource.test.ts index 99c08ac73f5..2fe42ea2da0 100644 --- a/public/app/plugins/datasource/tempo/datasource.test.ts +++ b/public/app/plugins/datasource/tempo/datasource.test.ts @@ -16,6 +16,12 @@ import { DEFAULT_LIMIT, TempoJsonData, TempoDatasource, TempoQuery } from './dat import mockJson from './mockJsonResponse.json'; describe('Tempo data source', () => { + // Mock the console error so that running the test suite doesnt throw the error + const origError = console.error; + const consoleErrorMock = jest.fn(); + afterEach(() => (console.error = origError)); + beforeEach(() => (console.error = consoleErrorMock)); + it('returns empty response when traceId is empty', async () => { const ds = new TempoDatasource(defaultSettings); const response = await lastValueFrom( diff --git a/public/app/plugins/datasource/zipkin/QueryField.test.tsx b/public/app/plugins/datasource/zipkin/QueryField.test.tsx index 76bd62d50c0..9ab560129b4 100644 --- a/public/app/plugins/datasource/zipkin/QueryField.test.tsx +++ b/public/app/plugins/datasource/zipkin/QueryField.test.tsx @@ -7,8 +7,9 @@ import { ZipkinQueryField, useLoadOptions, useServices } from './QueryField'; import { ZipkinQuery } from './types'; describe('QueryField', () => { - it('renders properly', () => { + it('renders properly', async () => { const ds = {} as ZipkinDatasource; + render( { /> ); - expect(screen.getByText(/1234/i)).toBeInTheDocument(); - expect(screen.getByText(/Traces/i)).toBeInTheDocument(); + expect(await screen.findByText(/1234/i)).toBeInTheDocument(); + expect(await screen.findByText(/Traces/i)).toBeInTheDocument(); }); }); diff --git a/public/app/plugins/panel/nodeGraph/NodeGraph.test.tsx b/public/app/plugins/panel/nodeGraph/NodeGraph.test.tsx index 58fa20e928f..e6bdf8380c7 100644 --- a/public/app/plugins/panel/nodeGraph/NodeGraph.test.tsx +++ b/public/app/plugins/panel/nodeGraph/NodeGraph.test.tsx @@ -3,6 +3,7 @@ import { render, screen, fireEvent, waitFor, getByText } from '@testing-library/ import userEvent from '@testing-library/user-event'; import { NodeGraph } from './NodeGraph'; import { makeEdgesDataFrame, makeNodesDataFrame } from './utils'; +import { act } from 'react-dom/test-utils'; jest.mock('react-use/lib/useMeasure', () => { return { @@ -14,6 +15,11 @@ jest.mock('react-use/lib/useMeasure', () => { }); describe('NodeGraph', () => { + const origError = console.error; + const consoleErrorMock = jest.fn(); + afterEach(() => (console.error = origError)); + beforeEach(() => (console.error = consoleErrorMock)); + it('shows no data message without any data', async () => { render( []} />); @@ -80,11 +86,15 @@ describe('NodeGraph', () => { const node = await screen.findByLabelText(/Node: service:0/); // This shows warning because there is no position for the click. We cannot add any because we use pageX/Y in the // context menu which is experimental (but supported) property and userEvents does not seem to support that - userEvent.click(node); + act(() => { + userEvent.click(node); + }); await screen.findByText(/Node traces/); const edge = await screen.findByLabelText(/Edge from/); - userEvent.click(edge); + act(() => { + userEvent.click(edge); + }); await screen.findByText(/Edge traces/); }); @@ -173,7 +183,9 @@ describe('NodeGraph', () => { expect(node).toBeInTheDocument(); const marker = await screen.findByLabelText(/Hidden nodes marker: 3/); - userEvent.click(marker); + act(() => { + userEvent.click(marker); + }); expect(screen.queryByLabelText(/Node: service:0/)).not.toBeInTheDocument(); expect(screen.getByLabelText(/Node: service:4/)).toBeInTheDocument(); diff --git a/yarn.lock b/yarn.lock index b2501f2b85c..d9669eb6682 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4580,7 +4580,6 @@ __metadata: "@types/prop-types": ^15 "@types/react": 17.0.39 "@types/react-icons": 2.2.7 - "@types/recompose": 0.30.10 "@types/reselect": 2.2.0 "@types/tinycolor2": 1.4.3 chance: ^1.0.10 @@ -4601,7 +4600,6 @@ __metadata: react: 17.0.2 react-dom: 17.0.2 react-icons: 2.2.7 - recompose: 0.30.0 reselect: 4.1.5 sinon: 13.0.1 tinycolor2: 1.4.2 @@ -10681,15 +10679,6 @@ __metadata: languageName: node linkType: hard -"@types/recompose@npm:0.30.10": - version: 0.30.10 - resolution: "@types/recompose@npm:0.30.10" - dependencies: - "@types/react": "*" - checksum: 2b58b9e75841c8f2ceb5b40974e958a815bd6a606869fb926ae27e40d9b3ef17ab6697f63d60a367e31fb2008bf35f74eaa6c6a9cd542f857e97568e8ebe322d - languageName: node - linkType: hard - "@types/redux-mock-store@npm:1.0.3": version: 1.0.3 resolution: "@types/redux-mock-store@npm:1.0.3" @@ -12912,7 +12901,7 @@ __metadata: languageName: node linkType: hard -"asap@npm:^2.0.0, asap@npm:~2.0.3": +"asap@npm:^2.0.0": version: 2.0.6 resolution: "asap@npm:2.0.6" checksum: b296c92c4b969e973260e47523207cd5769abd27c245a68c26dc7a0fe8053c55bb04360237cb51cab1df52be939da77150ace99ad331fb7fb13b3423ed73ff3d @@ -14469,13 +14458,6 @@ __metadata: languageName: node linkType: hard -"change-emitter@npm:^0.1.2": - version: 0.1.6 - resolution: "change-emitter@npm:0.1.6" - checksum: 0ed494ba9901ca56ea6f942668fd294465c334a9a0981dca96da5aea5e387c0023a630d7c658c1b532d203db54c928ddca2564e434b4a8b7f6d39155d09db255 - languageName: node - linkType: hard - "char-regex@npm:^1.0.2": version: 1.0.2 resolution: "char-regex@npm:1.0.2" @@ -15601,13 +15583,6 @@ __metadata: languageName: node linkType: hard -"core-js@npm:^1.0.0": - version: 1.2.7 - resolution: "core-js@npm:1.2.7" - checksum: 0b76371bfa98708351cde580f9287e2360d2209920e738ae950ae74ad08639a2e063541020bf666c28778956fc356ed9fe56d962129c88a87a6a4a0612526c75 - languageName: node - linkType: hard - "core-js@npm:^2.0.0, core-js@npm:^2.4.0": version: 2.6.12 resolution: "core-js@npm:2.6.12" @@ -17792,7 +17767,7 @@ __metadata: languageName: node linkType: hard -"encoding@npm:^0.1.11, encoding@npm:^0.1.12": +"encoding@npm:^0.1.12": version: 0.1.13 resolution: "encoding@npm:0.1.13" dependencies: @@ -19078,21 +19053,6 @@ __metadata: languageName: node linkType: hard -"fbjs@npm:^0.8.1": - version: 0.8.18 - resolution: "fbjs@npm:0.8.18" - dependencies: - core-js: ^1.0.0 - isomorphic-fetch: ^2.1.1 - loose-envify: ^1.0.0 - object-assign: ^4.1.0 - promise: ^7.1.1 - setimmediate: ^1.0.5 - ua-parser-js: ^0.7.30 - checksum: 668731b946a765908c9cbe51d5160f973abb78004b3d122587c3e930e3e1ddcc0ce2b17f2a8637dc9d733e149aa580f8d3035a35cc2d3bc78b78f1b19aab90e2 - languageName: node - linkType: hard - "fd-slicer@npm:~1.1.0": version: 1.1.0 resolution: "fd-slicer@npm:1.1.0" @@ -20989,13 +20949,6 @@ __metadata: languageName: node linkType: hard -"hoist-non-react-statics@npm:^2.3.1": - version: 2.5.5 - resolution: "hoist-non-react-statics@npm:2.5.5" - checksum: ee2d05e5c7e1398ad84a15b0327f66bd78f38a8e0015e852f954b36434e32eb7e942d5357505020a3a1147f247b165bf1e69d72393e3accab67cafdafeb86230 - languageName: node - linkType: hard - "hosted-git-info@npm:^2.1.4": version: 2.8.9 resolution: "hosted-git-info@npm:2.8.9" @@ -22503,7 +22456,7 @@ __metadata: languageName: node linkType: hard -"is-stream@npm:^1.0.1, is-stream@npm:^1.1.0": +"is-stream@npm:^1.1.0": version: 1.1.0 resolution: "is-stream@npm:1.1.0" checksum: 063c6bec9d5647aa6d42108d4c59723d2bd4ae42135a2d4db6eadbd49b7ea05b750fd69d279e5c7c45cf9da753ad2c00d8978be354d65aa9f6bb434969c6a2ae @@ -22703,16 +22656,6 @@ __metadata: languageName: node linkType: hard -"isomorphic-fetch@npm:^2.1.1": - version: 2.2.1 - resolution: "isomorphic-fetch@npm:2.2.1" - dependencies: - node-fetch: ^1.0.1 - whatwg-fetch: ">=0.10.0" - checksum: bb5daa7c3785d6742f4379a81e55b549a469503f7c9bf9411b48592e86632cf5e8fe8ea878dba185c0f33eb7c510c23abdeb55aebfdf5d3c70f031ced68c5424 - languageName: node - linkType: hard - "isstream@npm:~0.1.2": version: 0.1.2 resolution: "isstream@npm:0.1.2" @@ -26448,16 +26391,6 @@ __metadata: languageName: node linkType: hard -"node-fetch@npm:^1.0.1": - version: 1.7.3 - resolution: "node-fetch@npm:1.7.3" - dependencies: - encoding: ^0.1.11 - is-stream: ^1.0.1 - checksum: 3bb0528c05d541316ebe52770d71ee25a6dce334df4231fd55df41a644143e07f068637488c18a5b0c43f05041dbd3346752f9e19b50df50569a802484544d5b - languageName: node - linkType: hard - "node-fetch@npm:^2.6.1": version: 2.6.5 resolution: "node-fetch@npm:2.6.5" @@ -29509,15 +29442,6 @@ __metadata: languageName: node linkType: hard -"promise@npm:^7.1.1": - version: 7.3.1 - resolution: "promise@npm:7.3.1" - dependencies: - asap: ~2.0.3 - checksum: 475bb069130179fbd27ed2ab45f26d8862376a137a57314cf53310bdd85cc986a826fd585829be97ebc0aaf10e9d8e68be1bfe5a4a0364144b1f9eedfa940cf1 - languageName: node - linkType: hard - "prompts@npm:^2.0.1, prompts@npm:^2.4.0, prompts@npm:^2.4.2": version: 2.4.2 resolution: "prompts@npm:2.4.2" @@ -30701,7 +30625,7 @@ __metadata: languageName: node linkType: hard -"react-lifecycles-compat@npm:^3.0.2, react-lifecycles-compat@npm:^3.0.4": +"react-lifecycles-compat@npm:^3.0.4": version: 3.0.4 resolution: "react-lifecycles-compat@npm:3.0.4" checksum: a904b0fc0a8eeb15a148c9feb7bc17cec7ef96e71188280061fc340043fd6d8ee3ff233381f0e8f95c1cf926210b2c4a31f38182c8f35ac55057e453d6df204f @@ -31397,22 +31321,6 @@ __metadata: languageName: node linkType: hard -"recompose@npm:0.30.0": - version: 0.30.0 - resolution: "recompose@npm:0.30.0" - dependencies: - "@babel/runtime": ^7.0.0 - change-emitter: ^0.1.2 - fbjs: ^0.8.1 - hoist-non-react-statics: ^2.3.1 - react-lifecycles-compat: ^3.0.2 - symbol-observable: ^1.0.4 - peerDependencies: - react: ^0.14.0 || ^15.0.0 || ^16.0.0 - checksum: 18e58252336d0628b22db1e38407d32e836648e6d5c9453ba37c9f8030138b3429ee3952b053a13b60311f8b60893b207a761466bb293083542db0cf317b7a41 - languageName: node - linkType: hard - "recursive-readdir@npm:^2.2.2": version: 2.2.2 resolution: "recursive-readdir@npm:2.2.2" @@ -32707,7 +32615,7 @@ __metadata: languageName: node linkType: hard -"setimmediate@npm:^1.0.4, setimmediate@npm:^1.0.5": +"setimmediate@npm:^1.0.4": version: 1.0.5 resolution: "setimmediate@npm:1.0.5" checksum: c9a6f2c5b51a2dabdc0247db9c46460152ffc62ee139f3157440bd48e7c59425093f42719ac1d7931f054f153e2d26cf37dfeb8da17a794a58198a2705e527fd @@ -34213,13 +34121,6 @@ __metadata: languageName: node linkType: hard -"symbol-observable@npm:^1.0.4": - version: 1.2.0 - resolution: "symbol-observable@npm:1.2.0" - checksum: 48ffbc22e3d75f9853b3ff2ae94a44d84f386415110aea5effc24d84c502e03a4a6b7a8f75ebaf7b585780bda34eb5d6da3121f826a6f93398429d30032971b6 - languageName: node - linkType: hard - "symbol-tree@npm:^3.2.2, symbol-tree@npm:^3.2.4": version: 3.2.4 resolution: "symbol-tree@npm:3.2.4" @@ -35457,13 +35358,6 @@ __metadata: languageName: node linkType: hard -"ua-parser-js@npm:^0.7.30": - version: 0.7.30 - resolution: "ua-parser-js@npm:0.7.30" - checksum: 19aad6e1b9e93cf3c57e139858811c2c94031ed769f1ac36dc3cda1e1e5d2c73c2e02ddf4f9e0ea140069f803315484f955488961172a6a01e9322b6c042ae98 - languageName: node - linkType: hard - "uglify-js@npm:3.4.x": version: 3.4.10 resolution: "uglify-js@npm:3.4.10" @@ -36851,7 +36745,7 @@ __metadata: languageName: node linkType: hard -"whatwg-fetch@npm:3.6.2, whatwg-fetch@npm:>=0.10.0": +"whatwg-fetch@npm:3.6.2": version: 3.6.2 resolution: "whatwg-fetch@npm:3.6.2" checksum: ee976b7249e7791edb0d0a62cd806b29006ad7ec3a3d89145921ad8c00a3a67e4be8f3fb3ec6bc7b58498724fd568d11aeeeea1f7827e7e1e5eae6c8a275afed