Skip to content

Commit

Permalink
Sort steps by timestamps in a TaskRun
Browse files Browse the repository at this point in the history
Currently, all injected steps appear before the regularly defined
steps in some views. Now they are sorted by the finishedAt timestamp
in each step's status, with ties broken by the startedAt timestamp.

The current order is not always correct, since some injected steps
actually run after regular steps. This ordering also causes some steps
to be incorrectly labelled as either skipped or failed.

The new order should be better, but it will still depend on the
precision of the timestamps: seconds might not be sufficient in all
cases. There are plans in 'pipeline' to increase the precision.

Fixes #1367
Cherrypicked for #1438
  • Loading branch information
Justin Kulikauskas committed May 26, 2020
1 parent e175359 commit b991cfc
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 2 deletions.
3 changes: 2 additions & 1 deletion packages/components/src/components/TaskTree/TaskTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ limitations under the License.
*/

import React, { Component } from 'react';
import { sortStepsByTimestamp } from '@tektoncd/dashboard-utils';
import Task from '../Task';

import './TaskTree.scss';
Expand Down Expand Up @@ -47,7 +48,7 @@ class TaskTree extends Component {
expanded={expanded}
onSelect={this.handleSelect}
reason={reason}
steps={steps}
steps={sortStepsByTimestamp(steps)}
succeeded={succeeded}
pipelineTaskName={pipelineTaskName}
/>
Expand Down
23 changes: 22 additions & 1 deletion packages/utils/src/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,34 @@ export function formatLabels(labelsRaw) {
return formattedLabelsToRender;
}

// Sorts the steps by finishedAt and startedAt timestamps
export function sortStepsByTimestamp(steps) {
return steps.sort((i, j) => {
const iFinishAt = new Date(i.stepStatus?.terminated?.finishedAt).getTime();
const jFinishAt = new Date(j.stepStatus?.terminated?.finishedAt).getTime();
const iStartedAt = new Date(i.stepStatus?.terminated?.startedAt).getTime();
const jStartedAt = new Date(j.stepStatus?.terminated?.startedAt).getTime();

if (!iFinishAt || !jFinishAt) {
return 0;
}
if (iFinishAt !== jFinishAt) {
return iFinishAt - jFinishAt;
}
if (!iStartedAt || !jStartedAt) {
return 0;
}
return iStartedAt - jStartedAt;
});
}

// Update the status of steps that follow a step with an error
export function updateUnexecutedSteps(steps) {
if (!steps) {
return steps;
}
let errorIndex = steps.length - 1;
return steps.map((step, index) => {
return sortStepsByTimestamp(steps).map((step, index) => {
// Update errorIndex
if (step.reason !== 'Completed') {
errorIndex = Math.min(index, errorIndex);
Expand Down
123 changes: 123 additions & 0 deletions packages/utils/src/utils/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
reorderSteps,
selectedTask,
selectedTaskRun,
sortStepsByTimestamp,
stepsStatus,
taskRunStep,
updateUnexecutedSteps
Expand Down Expand Up @@ -374,6 +375,128 @@ it('formatLabels', () => {
]);
});

it('sortStepsByTimestamp preserves order if no timestamps present', () => {
const steps = ['t', 'e', 's', 't'];
const want = ['t', 'e', 's', 't'];
const got = sortStepsByTimestamp(steps);
expect(got).toEqual(want);
});

it('sortStepsByTimestamp sorts by finishedAt', () => {
const step1 = {
id: 'step1',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z'
}
}
};
const step2 = {
id: 'step2',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:25:00Z'
}
}
};
const step3 = {
id: 'step3',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:35:00Z'
}
}
};
const steps = [step1, step2, step3];
const want = [step2, step1, step3];
const got = sortStepsByTimestamp(steps);
expect(got).toEqual(want);
});

it('sortStepsByTimestamp sorts by startedAt in a tie', () => {
const step1 = {
id: 'step1',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z',
startedAt: '2020-01-01T15:25:00Z'
}
}
};
const step2 = {
id: 'step2',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:25:00Z',
startedAt: '2020-01-01T15:20:00Z'
}
}
};
const step3 = {
id: 'step3',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z',
startedAt: '2020-01-01T15:30:00Z'
}
}
};
const steps = [step1, step2, step3];
const want = [step2, step1, step3];
const got = sortStepsByTimestamp(steps);
expect(got).toEqual(want);
});

it('sortStepsByTimestamp preserves order if invalid startedAt timestamp present', () => {
const step1 = {
id: 'step1',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z',
startedAt: '2020-01-01T15:25:00Z'
}
}
};
const step2 = {
id: 'step2',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z',
startedAt: 'NOTATIMESTAMP'
}
}
};
const steps = [step1, step2];
const want = [step1, step2];
const got = sortStepsByTimestamp(steps);
expect(got).toEqual(want);
});

it('sortStepsByTimestamp preserves order if startedAt timestamps equal', () => {
const step1 = {
id: 'step-b',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z',
startedAt: '2020-01-01T15:25:00Z'
}
}
};
const step2 = {
id: 'step-a',
stepStatus: {
terminated: {
finishedAt: '2020-01-01T15:30:00Z',
startedAt: '2020-01-01T15:25:00Z'
}
}
};
const steps = [step1, step2];
const want = [step1, step2];
const got = sortStepsByTimestamp(steps);
expect(got).toEqual(want);
});

it('updateUnexecutedSteps no steps', () => {
const steps = [];
const wantUpdatedSteps = [];
Expand Down

0 comments on commit b991cfc

Please sign in to comment.