Skip to content

Commit bfac14f

Browse files
anwesha-palit-redhatvikram-raj
authored andcommitted
fix: resolved sorting issue in PLR
1 parent 719f9b4 commit bfac14f

File tree

3 files changed

+275
-1
lines changed

3 files changed

+275
-1
lines changed

src/components/pipelineRuns-list/usePipelineRunsColumns.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '../../consts';
1010
import { PipelineRunKind } from '../../types';
1111
import { tableColumnClasses } from './PipelineRunsRow';
12+
import { sortPipelineRunsByDuration } from '../pipelines-details/pipeline-step-utils';
1213

1314
const usePipelineRunsColumns = (
1415
namespace: string,
@@ -76,7 +77,7 @@ const usePipelineRunsColumns = (
7677
{
7778
id: 'duration',
7879
title: t('Duration'),
79-
sort: 'status.completionTime',
80+
sort: sortPipelineRunsByDuration,
8081
transforms: [sortable],
8182
props: { className: tableColumnClasses.duration },
8283
},
Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
import { ComputedStatus, PipelineRunKind } from '../../../types';
2+
import { pipelineRunStatus } from '../../utils/pipeline-filter-reducer';
3+
import { sortPipelineRunsByDuration } from '../pipeline-step-utils';
4+
5+
// Mock the pipeline-filter-reducer module
6+
jest.mock('../../utils/pipeline-filter-reducer', () => ({
7+
pipelineRunStatus: jest.fn(),
8+
}));
9+
10+
const mockPipelineRunStatus = pipelineRunStatus as jest.MockedFunction<
11+
typeof pipelineRunStatus
12+
>;
13+
14+
describe('sortPipelineRunsByDuration', () => {
15+
const createMockPipelineRun = (
16+
name: string,
17+
startTime?: string,
18+
completionTime?: string,
19+
status?: ComputedStatus,
20+
): PipelineRunKind => ({
21+
metadata: {
22+
name,
23+
namespace: 'test-namespace',
24+
},
25+
spec: {},
26+
status: {
27+
pipelineSpec: {
28+
tasks: [],
29+
},
30+
...(startTime && { startTime }),
31+
...(completionTime && { completionTime }),
32+
...(status && { status }),
33+
},
34+
});
35+
36+
beforeEach(() => {
37+
jest.clearAllMocks();
38+
mockPipelineRunStatus.mockReturnValue(ComputedStatus.Succeeded);
39+
});
40+
41+
describe('ascending order', () => {
42+
it('should sort pipeline runs by duration in ascending order', () => {
43+
const pipelineRuns: PipelineRunKind[] = [
44+
createMockPipelineRun(
45+
'run-long',
46+
'2023-01-01T10:00:00Z',
47+
'2023-01-01T10:10:00Z', // 10 minutes
48+
),
49+
createMockPipelineRun(
50+
'run-short',
51+
'2023-01-01T10:00:00Z',
52+
'2023-01-01T10:05:00Z', // 5 minutes
53+
),
54+
createMockPipelineRun(
55+
'run-very-short',
56+
'2023-01-01T10:00:00Z',
57+
'2023-01-01T10:00:30Z', // 30 seconds
58+
),
59+
createMockPipelineRun(
60+
'run-medium',
61+
'2023-01-01T10:00:00Z',
62+
'2023-01-01T10:07:00Z', // 7 minutes
63+
),
64+
];
65+
66+
const result = sortPipelineRunsByDuration(pipelineRuns, 'asc');
67+
68+
expect(result[0].metadata.name).toBe('run-very-short');
69+
expect(result[1].metadata.name).toBe('run-short');
70+
expect(result[2].metadata.name).toBe('run-medium');
71+
expect(result[3].metadata.name).toBe('run-long');
72+
});
73+
74+
it('should handle running pipeline runs by using current time for duration calculation', () => {
75+
const now = new Date('2023-01-01T10:30:00Z');
76+
jest.spyOn(Date, 'now').mockReturnValue(now.getTime());
77+
mockPipelineRunStatus.mockReturnValue(ComputedStatus.Running);
78+
79+
const pipelineRuns: PipelineRunKind[] = [
80+
createMockPipelineRun(
81+
'run-completed',
82+
'2023-01-01T10:00:00Z',
83+
'2023-01-01T10:05:00Z', // 5 minutes
84+
),
85+
createMockPipelineRun(
86+
'run-running',
87+
'2023-01-01T10:00:00Z', // 30 minutes from start to now
88+
),
89+
];
90+
91+
const result = sortPipelineRunsByDuration(pipelineRuns, 'asc');
92+
93+
expect(result[0].metadata.name).toBe('run-completed');
94+
expect(result[1].metadata.name).toBe('run-running');
95+
96+
jest.restoreAllMocks();
97+
});
98+
});
99+
100+
describe('descending order', () => {
101+
it('should sort pipeline runs by duration in descending order', () => {
102+
const pipelineRuns: PipelineRunKind[] = [
103+
createMockPipelineRun(
104+
'run-short',
105+
'2023-01-01T10:00:00Z',
106+
'2023-01-01T10:05:00Z', // 5 minutes
107+
),
108+
createMockPipelineRun(
109+
'run-long',
110+
'2023-01-01T10:00:00Z',
111+
'2023-01-01T10:10:00Z', // 10 minutes
112+
),
113+
createMockPipelineRun(
114+
'run-medium',
115+
'2023-01-01T10:00:00Z',
116+
'2023-01-01T10:07:00Z', // 7 minutes
117+
),
118+
];
119+
120+
const result = sortPipelineRunsByDuration(pipelineRuns, 'desc');
121+
122+
expect(result[0].metadata.name).toBe('run-long');
123+
expect(result[1].metadata.name).toBe('run-medium');
124+
expect(result[2].metadata.name).toBe('run-short');
125+
});
126+
});
127+
128+
describe('edge cases', () => {
129+
it('should handle pipeline runs without completion time (but not running)', () => {
130+
mockPipelineRunStatus.mockReturnValue(ComputedStatus.Failed);
131+
132+
const pipelineRuns: PipelineRunKind[] = [
133+
createMockPipelineRun('run-no-completion', '2023-01-01T10:00:00Z'),
134+
createMockPipelineRun(
135+
'run-normal',
136+
'2023-01-01T10:00:00Z',
137+
'2023-01-01T10:05:00Z',
138+
),
139+
];
140+
141+
const result = sortPipelineRunsByDuration(pipelineRuns, 'asc');
142+
143+
expect(result[0].metadata.name).toBe('run-no-completion');
144+
expect(result[1].metadata.name).toBe('run-normal');
145+
});
146+
147+
it('should sort by name when durations are equal', () => {
148+
const pipelineRuns: PipelineRunKind[] = [
149+
createMockPipelineRun(
150+
'run-z',
151+
'2023-01-01T10:00:00Z',
152+
'2023-01-01T10:05:00Z', // 5 minutes
153+
),
154+
createMockPipelineRun(
155+
'run-a',
156+
'2023-01-01T10:00:00Z',
157+
'2023-01-01T10:05:00Z', // 5 minutes
158+
),
159+
createMockPipelineRun(
160+
'run-m',
161+
'2023-01-01T10:00:00Z',
162+
'2023-01-01T10:05:00Z', // 5 minutes
163+
),
164+
];
165+
166+
const result = sortPipelineRunsByDuration(pipelineRuns, 'asc');
167+
168+
expect(result[0].metadata.name).toBe('run-a');
169+
expect(result[1].metadata.name).toBe('run-m');
170+
expect(result[2].metadata.name).toBe('run-z');
171+
});
172+
173+
it('should not mutate the original array', () => {
174+
const pipelineRuns: PipelineRunKind[] = [
175+
createMockPipelineRun(
176+
'run-b',
177+
'2023-01-01T10:00:00Z',
178+
'2023-01-01T10:10:00Z',
179+
),
180+
createMockPipelineRun(
181+
'run-a',
182+
'2023-01-01T10:00:00Z',
183+
'2023-01-01T10:05:00Z',
184+
),
185+
];
186+
187+
const originalOrder = pipelineRuns.map((run) => run.metadata.name);
188+
sortPipelineRunsByDuration(pipelineRuns, 'asc');
189+
190+
// Original array should remain unchanged
191+
expect(pipelineRuns.map((run) => run.metadata.name)).toEqual(
192+
originalOrder,
193+
);
194+
});
195+
196+
it('should handle empty array', () => {
197+
const result = sortPipelineRunsByDuration([], 'asc');
198+
expect(result).toEqual([]);
199+
});
200+
201+
it('should handle single pipeline run', () => {
202+
const pipelineRuns: PipelineRunKind[] = [
203+
createMockPipelineRun(
204+
'single-run',
205+
'2023-01-01T10:00:00Z',
206+
'2023-01-01T10:05:00Z',
207+
),
208+
];
209+
210+
const result = sortPipelineRunsByDuration(pipelineRuns, 'asc');
211+
212+
expect(result).toHaveLength(1);
213+
expect(result[0].metadata.name).toBe('single-run');
214+
});
215+
});
216+
217+
describe('duration calculation edge cases', () => {
218+
it('should handle pipeline runs with completion time before start time', () => {
219+
const pipelineRuns: PipelineRunKind[] = [
220+
createMockPipelineRun(
221+
'run-time-anomaly',
222+
'2023-01-01T10:10:00Z', // start time after completion
223+
'2023-01-01T10:05:00Z', // completion time before start
224+
),
225+
createMockPipelineRun(
226+
'run-normal',
227+
'2023-01-01T10:00:00Z',
228+
'2023-01-01T10:05:00Z',
229+
),
230+
];
231+
232+
const result = sortPipelineRunsByDuration(pipelineRuns, 'asc');
233+
234+
// Should handle negative duration gracefully
235+
expect(result).toHaveLength(2);
236+
});
237+
});
238+
});

src/components/pipelines-details/pipeline-step-utils.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { ComputedStatus } from '../../types';
22
import { calculateDuration } from '../utils/pipeline-utils';
3+
import { PipelineRunKind } from '../../types';
4+
import { pipelineRunStatus } from '../utils/pipeline-filter-reducer';
35

46
enum TerminatedReasons {
57
Completed = 'Completed',
@@ -97,3 +99,36 @@ export const createStepStatus = (
9799
status: stepRunStatus,
98100
};
99101
};
102+
103+
export const sortPipelineRunsByDuration = (
104+
pipelineRuns: PipelineRunKind[],
105+
direction: 'asc' | 'desc',
106+
) => {
107+
return [...pipelineRuns].sort((a: PipelineRunKind, b: PipelineRunKind) => {
108+
const getDurationMs = (run: PipelineRunKind): number => {
109+
const startTime = run?.status?.startTime ?? null;
110+
const completionTime = run?.status?.completionTime ?? null;
111+
112+
if (
113+
!startTime ||
114+
(!completionTime && pipelineRunStatus(run) !== 'Running')
115+
) {
116+
return -1;
117+
}
118+
const start = new Date(startTime).getTime();
119+
const end = completionTime
120+
? new Date(completionTime).getTime()
121+
: new Date().getTime();
122+
return end - start;
123+
};
124+
125+
const durationA = getDurationMs(a);
126+
const durationB = getDurationMs(b);
127+
128+
if (durationA === durationB) {
129+
return (a?.metadata?.name || '').localeCompare(b?.metadata?.name || '');
130+
}
131+
132+
return direction === 'asc' ? durationA - durationB : durationB - durationA;
133+
});
134+
};

0 commit comments

Comments
 (0)