Skip to content

Commit 667fd80

Browse files
chmouelvdemeester
authored andcommitted
fix: Stabilize sorting logic for task logs and status
The sorting logic for task logs and status did not have a stable tie-breaker when completion or start times were identical. This resulted in inconsistent sorting order for tasks with the same timestamp. The `taskInfoSorter.Less` function was updated to sort by task name alphabetically when `CompletionTime` is the same. The `taskrunList.Less` function was updated to sort by `PipelineTaskName` in reverse alphabetical order when `StartTime` is the same. This ensures a consistent and predictable sorting order in all scenarios. Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com> fix: More Stabilize task log snippet sorting Modified the sorting logic for task log snippets to ensure consistent ordering. The changes prioritize sorting by completion time (earliest first), and then by name for tasks with the same completion time. Running tasks, identified by a nil CompletionTime, are now consistently placed at the end of the sorted list. This addresses edge cases where nil completion times could lead to unpredictable sorting behavior. Revert "fix: Stabilize task log snippet sorting" This reverts commit 84a257f20cd70fa2657a88b2286ab555eb753522.
1 parent ce269cc commit 667fd80

File tree

4 files changed

+65
-0
lines changed

4 files changed

+65
-0
lines changed

pkg/sort/task_log_snippets.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ func (s taskInfoSorter) Swap(i, j int) {
1515
}
1616

1717
func (s taskInfoSorter) Less(i, j int) bool {
18+
if s[i].CompletionTime.Equal(s[j].CompletionTime) {
19+
return s[i].Name < s[j].Name
20+
}
1821
return s[i].CompletionTime.Before(s[j].CompletionTime)
1922
}
2023

pkg/sort/task_log_snippets_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,26 @@ func TestTaskInfos(t *testing.T) {
3838
},
3939
want: "before",
4040
},
41+
{
42+
name: "same completion time sorts by name",
43+
args: args{
44+
taskinfos: map[string]pacv1alpha1.TaskInfos{
45+
"task-zebra": {
46+
Name: "zebra",
47+
CompletionTime: &metav1.Time{
48+
Time: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC),
49+
},
50+
},
51+
"task-alpha": {
52+
Name: "alpha",
53+
CompletionTime: &metav1.Time{
54+
Time: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC),
55+
},
56+
},
57+
},
58+
},
59+
want: "alpha",
60+
},
4161
}
4262
for _, tt := range tests {
4363
t.Run(tt.name, func(t *testing.T) {

pkg/sort/task_status.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ func (trs taskrunList) Less(i, j int) bool {
3838
return true
3939
}
4040

41+
if trs[i].Status.StartTime.Equal(trs[j].Status.StartTime) {
42+
return trs[i].PipelineTaskName > trs[j].PipelineTaskName
43+
}
4144
return trs[j].Status.StartTime.Before(trs[i].Status.StartTime)
4245
}
4346

pkg/sort/task_status_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@ package sort
33
import (
44
"regexp"
55
"testing"
6+
"time"
67

78
"github.com/openshift-pipelines/pipelines-as-code/pkg/consoleui"
89
"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
910
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
1011
tektontest "github.com/openshift-pipelines/pipelines-as-code/pkg/test/tekton"
1112
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
1213
"gotest.tools/v3/assert"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1315
)
1416

1517
func TestStatusTmpl(t *testing.T) {
@@ -88,3 +90,40 @@ func TestStatusTmpl(t *testing.T) {
8890
})
8991
}
9092
}
93+
94+
func TestStatusTmplSameStartTime(t *testing.T) {
95+
flattedTmpl := `{{- range $taskrun := .TaskRunList }}{{- $taskrun.ConsoleLogURL }}{{- end }}`
96+
wantRegexp := regexp.MustCompile(".*alpha.*zebra.*")
97+
98+
// Use a single shared time to ensure Equal() returns true
99+
sharedTime := &metav1.Time{Time: time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)}
100+
101+
prTaskRunStatus := map[string]*tektonv1.PipelineRunTaskRunStatus{
102+
"zebra": {
103+
PipelineTaskName: "zebra",
104+
Status: &tektonv1.TaskRunStatus{
105+
TaskRunStatusFields: tektonv1.TaskRunStatusFields{
106+
StartTime: sharedTime,
107+
},
108+
},
109+
},
110+
"alpha": {
111+
PipelineTaskName: "alpha",
112+
Status: &tektonv1.TaskRunStatus{
113+
TaskRunStatusFields: tektonv1.TaskRunStatusFields{
114+
StartTime: sharedTime,
115+
},
116+
},
117+
},
118+
}
119+
120+
config := &info.ProviderConfig{
121+
TaskStatusTMPL: flattedTmpl,
122+
}
123+
runs := params.New()
124+
runs.Clients.SetConsoleUI(consoleui.FallBackConsole{})
125+
pr := &tektonv1.PipelineRun{}
126+
output, err := TaskStatusTmpl(pr, prTaskRunStatus, runs, config)
127+
assert.NilError(t, err)
128+
assert.Assert(t, wantRegexp.MatchString(output), "%s != %s", output, wantRegexp.String())
129+
}

0 commit comments

Comments
 (0)