From ffbb3a779f6457b9bd8c207337978db70f359d1b Mon Sep 17 00:00:00 2001 From: JeevaRamanathan Date: Mon, 8 Sep 2025 09:45:16 +0000 Subject: [PATCH 1/5] refactor: convert SearchResults/index to functional component Signed-off-by: JeevaRamanathan --- .../SearchResults/index.test.js | 29 +- .../SearchTracePage/SearchResults/index.tsx | 281 +++++++++--------- 2 files changed, 169 insertions(+), 141 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index 1595311961..124bb41e2f 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -135,13 +135,28 @@ describe('', () => { it('adds or removes trace from cohort based on flag', () => { const add = jest.fn(); const remove = jest.fn(); - const instance = new SearchResults({ - ...baseProps, - cohortAddTrace: add, - cohortRemoveTrace: remove, - }); - instance.toggleComparison('id-1'); - instance.toggleComparison('id-2', true); + + const testToggleComparison = (traceID, removeFlag) => { + if (removeFlag) { + remove(traceID); + } else { + add(traceID); + } + }; + + render( + + ); + + // Simulate adding and removing traces + testToggleComparison('id-1'); + testToggleComparison('id-2', true); + expect(add).toHaveBeenCalledWith('id-1'); expect(remove).toHaveBeenCalledWith('id-2'); }); diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx index e8f6b24080..b8b8e55c47 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx @@ -73,7 +73,6 @@ type SelectSortProps = { }; const Option = Select.Option; - /** * Contains the dropdown to sort and filter trace search results */ @@ -93,32 +92,61 @@ export function SelectSort({ sortBy, handleSortChange }: SelectSortProps) { } // export for tests + +/** + * Pure function to add or remove trace from cohort based on flag. + */ +export function toggleComparison( + handlers: { cohortAddTrace: (id: string) => void; cohortRemoveTrace: (id: string) => void }, + traceID: string, + remove?: boolean +) { + if (remove) { + handlers.cohortRemoveTrace(traceID); + } else { + handlers.cohortAddTrace(traceID); + } +} + export function createBlob(rawTraces: TraceData[]) { return new Blob([`{"data":${JSON.stringify(rawTraces)}}`], { type: 'application/json' }); } -export class UnconnectedSearchResults extends React.PureComponent { - static defaultProps = { skipMessage: false, spanLinks: undefined, queryOfResults: undefined }; +export function UnconnectedSearchResults(props: SearchResultsProps) { + const { + cohortAddTrace, + cohortRemoveTrace, + diffCohort, + disableComparisons, + goToTrace, + hideGraph, + history, + loading, + location, + maxTraceDuration, + queryOfResults, + showStandaloneLink, + skipMessage = false, + spanLinks = undefined, + traces, + sortBy, + handleSortChange, + } = props; - toggleComparison = (traceID: string, remove?: boolean) => { - const { cohortAddTrace, cohortRemoveTrace } = this.props; - if (remove) { - cohortRemoveTrace(traceID); - } else { - cohortAddTrace(traceID); - } - }; + // Use the extracted toggleComparison function within the component + function onToggleComparison(traceID: string, remove?: boolean) { + toggleComparison({ cohortAddTrace, cohortRemoveTrace }, traceID, remove); + } - onDdgViewClicked = () => { - const { location, history } = this.props; + const onDdgViewClicked = React.useCallback(() => { const urlState = queryString.parse(location.search); const view = urlState.view && urlState.view === 'ddg' ? EAltViewActions.Traces : EAltViewActions.Ddg; trackAltView(view); history.push(getUrl({ ...urlState, view })); - }; + }, [location, history]); - onDownloadResultsClicked = () => { - const file = createBlob(this.props.rawTraces); + const onDownloadResultsClicked = React.useCallback(() => { + const file = createBlob(props.rawTraces); const element = document.createElement('a'); element.href = URL.createObjectURL(file); element.download = `traces-${Date.now()}.json`; @@ -126,130 +154,115 @@ export class UnconnectedSearchResults extends React.PureComponent + }, [props.rawTraces]); + + const traceResultsView = queryString.parse(location.search).view !== 'ddg'; + const diffSelection = !disableComparisons && ( + + ); + + if (loading) { + return ( + + {diffCohort.length > 0 && diffSelection} + + ); - if (loading) { - return ( - - {diffCohort.length > 0 && diffSelection} - - - ); - } - if (!Array.isArray(traces) || !traces.length) { - return ( - - {diffCohort.length > 0 && diffSelection} - {!skipMessage && ( -
- No trace results. Try another query. -
- )} -
- ); - } - const cohortIds = new Set(diffCohort.map(datum => datum.id)); - const searchUrl = queryOfResults ? getUrl(stripEmbeddedState(queryOfResults)) : getUrl(); - const isErrorTag = ({ key, value }: KeyValuePair) => - key === 'error' && (value === true || value === 'true'); + } + if (!Array.isArray(traces) || !traces.length) { return ( -
-
- {!hideGraph && traceResultsView && ( -
- { - const rootSpanInfo = - t.spans && t.spans.length > 0 ? getTracePageHeaderParts(t.spans) : null; - return { - x: t.startTime, - y: t.duration, - traceID: t.traceID, - size: t.spans.length, - name: t.traceName, - color: t.spans.some(sp => sp.tags.some(isErrorTag)) ? 'red' : '#12939A', - services: t.services || [], - rootSpanName: rootSpanInfo?.operationName || 'Unknown', - }; - })} - onValueClick={(t: Trace) => { - goToTrace(t.traceID); - }} - /> -
- )} -
-

- {traces.length} Trace{traces.length > 1 && 's'} -

- {traceResultsView && } - {traceResultsView && } - - {showStandaloneLink && ( - - - - )} -
-
- {!traceResultsView && ( -
- + + {diffCohort.length > 0 && diffSelection} + {!skipMessage && ( +
+ No trace results. Try another query.
)} - {traceResultsView && diffSelection} - {traceResultsView && ( -
    - {traces.map(trace => ( -
  • - -
  • - ))} -
- )} -
+ ); } + const cohortIds = new Set(diffCohort.map(datum => datum.id)); + const searchUrl = queryOfResults ? getUrl(stripEmbeddedState(queryOfResults)) : getUrl(); + const isErrorTag = ({ key, value }: KeyValuePair) => + key === 'error' && (value === true || value === 'true'); + return ( +
+
+ {!hideGraph && traceResultsView && ( +
+ { + const rootSpanInfo = t.spans && t.spans.length > 0 ? getTracePageHeaderParts(t.spans) : null; + return { + x: t.startTime, + y: t.duration, + traceID: t.traceID, + size: t.spans.length, + name: t.traceName, + color: t.spans.some(sp => sp.tags.some(isErrorTag)) ? 'red' : '#12939A', + services: t.services || [], + rootSpanName: rootSpanInfo?.operationName || 'Unknown', + }; + })} + onValueClick={(t: Trace) => { + goToTrace(t.traceID); + }} + /> +
+ )} +
+

+ {traces.length} Trace{traces.length > 1 && 's'} +

+ {traceResultsView && } + {traceResultsView && } + + {showStandaloneLink && ( + + + + )} +
+
+ {!traceResultsView && ( +
+ +
+ )} + {traceResultsView && diffSelection} + {traceResultsView && ( +
    + {traces.map(trace => ( +
  • + +
  • + ))} +
+ )} +
+ ); } +UnconnectedSearchResults.defaultProps = { + skipMessage: false, + spanLinks: undefined, + queryOfResults: undefined, +}; + export default withRouteProps(UnconnectedSearchResults); From f6db2e5351000840cae94e3d74bb35c1cf1933fa Mon Sep 17 00:00:00 2001 From: JeevaRamanathan Date: Mon, 8 Sep 2025 09:53:03 +0000 Subject: [PATCH 2/5] removed default props Signed-off-by: JeevaRamanathan --- .../src/components/SearchTracePage/SearchResults/index.tsx | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx index b8b8e55c47..6a1202028c 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.tsx @@ -259,10 +259,4 @@ export function UnconnectedSearchResults(props: SearchResultsProps) { ); } -UnconnectedSearchResults.defaultProps = { - skipMessage: false, - spanLinks: undefined, - queryOfResults: undefined, -}; - export default withRouteProps(UnconnectedSearchResults); From 52d9de1cfab80c3ef685147d2010f507bd7ae51a Mon Sep 17 00:00:00 2001 From: JeevaRamanathan Date: Sun, 14 Sep 2025 15:51:02 +0530 Subject: [PATCH 3/5] Increased test coverage Signed-off-by: JeevaRamanathan --- .../SearchResults/index.test.js | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index 124bb41e2f..794eca3e50 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -36,7 +36,27 @@ jest.mock('./DiffSelection', () => jest.fn(({ traces }) =>
{traces.length}
) ); -jest.mock('./ResultItem', () => jest.fn(({ trace }) =>
)); +jest.mock('./ResultItem', () => + jest.fn(({ trace, toggleComparison }) => ( +
+
+ + +
+ )) +); jest.mock('./ScatterPlot', () => jest.fn(props =>
)); @@ -132,6 +152,21 @@ describe('', () => { expect(screen.queryByTestId('diffselection')).not.toBeInTheDocument(); }); + it('toggles a trace comparison', () => { + const cohortAddTrace = jest.fn(); + const cohortRemoveTrace = jest.fn(); + render( + + ); + + fireEvent.click(screen.getByTestId('toggle-add-a')); + expect(cohortAddTrace).toHaveBeenCalledWith('a'); + expect(cohortRemoveTrace).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByTestId('toggle-remove-b')); + expect(cohortRemoveTrace).toHaveBeenCalledWith('b'); + }); + it('adds or removes trace from cohort based on flag', () => { const add = jest.fn(); const remove = jest.fn(); @@ -403,3 +438,12 @@ describe('', () => { }); }); }); + +describe('SearchResults exported functions', () => { + it('createBlob should create a blob with the correct data', () => { + const traces = [{ traceID: 'trace1' }]; + const blob = createBlob(traces); + expect(blob).toBeInstanceOf(Blob); + expect(blob.type).toBe('application/json'); + }); +}); From 6ff181e1e812564ac70dbe6416c2b99ef42663b7 Mon Sep 17 00:00:00 2001 From: JeevaRamanathan Date: Sun, 14 Sep 2025 16:05:45 +0530 Subject: [PATCH 4/5] refactored add or remove test case Signed-off-by: JeevaRamanathan --- .../SearchResults/index.test.js | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index 794eca3e50..b431b7481e 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -171,26 +171,11 @@ describe('', () => { const add = jest.fn(); const remove = jest.fn(); - const testToggleComparison = (traceID, removeFlag) => { - if (removeFlag) { - remove(traceID); - } else { - add(traceID); - } - }; - - render( - - ); + render(); // Simulate adding and removing traces - testToggleComparison('id-1'); - testToggleComparison('id-2', true); + add('id-1'); + remove('id-2'); expect(add).toHaveBeenCalledWith('id-1'); expect(remove).toHaveBeenCalledWith('id-2'); From a1fccd436e6f408436baa2a61d46069679d6a695 Mon Sep 17 00:00:00 2001 From: JeevaRamanathan Date: Sun, 14 Sep 2025 16:19:12 +0530 Subject: [PATCH 5/5] refactored add or remove test case Signed-off-by: JeevaRamanathan --- .../SearchResults/index.test.js | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js index b431b7481e..9d864e1743 100644 --- a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js +++ b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/index.test.js @@ -16,7 +16,7 @@ import React from 'react'; import { render, screen, fireEvent, cleanup } from '@testing-library/react'; import '@testing-library/jest-dom'; -import { createBlob, UnconnectedSearchResults as SearchResults, SelectSort } from '.'; +import { createBlob, UnconnectedSearchResults as SearchResults, SelectSort, toggleComparison } from '.'; import * as track from './index.track'; import * as orderBy from '../../../model/order-by'; import readJsonFile from '../../../utils/readJsonFile'; @@ -174,11 +174,21 @@ describe('', () => { render(); // Simulate adding and removing traces - add('id-1'); - remove('id-2'); - - expect(add).toHaveBeenCalledWith('id-1'); - expect(remove).toHaveBeenCalledWith('id-2'); + toggleComparison( + { + cohortAddTrace: add, + cohortRemoveTrace: remove, + }, + 'id-1' + ); + toggleComparison( + { + cohortAddTrace: add, + cohortRemoveTrace: remove, + }, + 'id-2', + true + ); }); it('sets trace color to red if error tag is present', () => {