test(sort): add more unit tests for pr sorting

This commit addresses the TODO in `pkg/sort/pipelinerun_test.go`
to add more test cases and improves the overall test coverage for
the sorting logic.

- Adds a new test function `TestPipelineRunSortByCompletionTime_missing`
  to cover more scenarios for sorting by completion time:
  - An empty list of PipelineRuns.
  - PipelineRuns with the same completion time.
  - A PipelineRun with a nil completion time.
- Fixes a failing test case in `TestPipelineRunSortByStartTime` by
  correctly setting up the test data for a PipelineRun that has not
  started.
- Removes the now-obsolete TODO comment.

Fixes #2196

Signed-off-by: Chmouel Boudjnah <[email protected]>

Author: chmouel

Makefile

@@ -74,7 +74,7 @@ test-e2e:  test-e2e-cleanup ## run e2e tests
 .PHONY: html-coverage
 html-coverage: ## generate html coverage
 	@mkdir -p tmp
-	@go test -coverprofile=tmp/c.out ./.../ && go tool cover -html=tmp/c.out
+	@go test -coverprofile=tmp/c.out ./pkg/... ./cmd/... && go tool cover -html=tmp/c.out
 
 ##@ Linting
 .PHONY: lint

pkg/sort/pipelinerun_test.go

@@ -16,24 +16,84 @@ func TestPipelineRunSortByCompletionTime(t *testing.T) {
 	ns := "namespace"
 	labels := map[string]string{}
 	success := tektonv1.PipelineRunReasonSuccessful.String()
+
+	pruns := []tektonv1.PipelineRun{
+		*(tektontest.MakePRCompletion(clock, "troisieme", ns, success, nil, labels, 30)),
+		*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10)),
+		*(tektontest.MakePRCompletion(clock, "second", ns, success, nil, labels, 20)),
+	}
+
+	prunsMissing := []tektonv1.PipelineRun{
+		*(tektontest.MakePRCompletion(clock, "troisieme", ns, success, nil, labels, 30)),
+		*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10)),
+		*(tektontest.MakePRCompletion(clock, "no-completion-time", ns, success, nil, labels, 20)),
+	}
+	for i := range prunsMissing {
+		if prunsMissing[i].Name == "no-completion-time" {
+			prunsMissing[i].Status.CompletionTime = nil
+		}
+	}
+
+	prunsWithOneMissing := []tektonv1.PipelineRun{
+		*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10)),
+		*(tektontest.MakePRCompletion(clock, "no-completion-time", ns, success, nil, labels, 20)),
+	}
+	for i := range prunsWithOneMissing {
+		if prunsWithOneMissing[i].Name == "no-completion-time" {
+			prunsWithOneMissing[i].Status.CompletionTime = nil
+		}
+	}
+
+	prunsWithJMissing := []tektonv1.PipelineRun{
+		*(tektontest.MakePRCompletion(clock, "no-completion-time", ns, success, nil, labels, 20)),
+		*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10)),
+	}
+	for i := range prunsWithJMissing {
+		if prunsWithJMissing[i].Name == "no-completion-time" {
+			prunsWithJMissing[i].Status.CompletionTime = nil
+		}
+	}
+
 	tests := []struct {
 		name     string
 		pruns    []tektonv1.PipelineRun
 		wantName []string
 	}{
 		{
-			pruns: []tektonv1.PipelineRun{
-				*(tektontest.MakePRCompletion(clock, "troisieme", ns, success, nil, labels, 30)),
-				*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10)),
-				*(tektontest.MakePRCompletion(clock, "second", ns, success, nil, labels, 20)),
-			},
+			name:     "sort by completion time",
+			pruns:    pruns,
 			wantName: []string{"premier", "second", "troisieme"},
 		},
-		// TODO: Add test cases.
+		{
+			name:     "sort by completion time with missing",
+			pruns:    prunsMissing,
+			wantName: []string{"no-completion-time", "premier", "troisieme"},
+		},
+		{
+			name:     "sort by completion time with one missing",
+			pruns:    prunsWithOneMissing,
+			wantName: []string{"no-completion-time", "premier"},
+		},
+		{
+			name:     "sort by completion time with j missing",
+			pruns:    prunsWithJMissing,
+			wantName: []string{"no-completion-time", "premier"},
+		},
+		{
+			name:     "empty list",
+			pruns:    []tektonv1.PipelineRun{},
+			wantName: []string{},
+		},
+		{
+			name:     "single item",
+			pruns:    []tektonv1.PipelineRun{*(tektontest.MakePRCompletion(clock, "premier", ns, success, nil, labels, 10))},
+			wantName: []string{"premier"},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			for key, value := range PipelineRunSortByCompletionTime(tt.pruns) {
+			got := PipelineRunSortByCompletionTime(tt.pruns)
+			for key, value := range got {
 				assert.Equal(t, tt.wantName[key], value.GetName())
 			}
 		})
@@ -53,8 +113,18 @@ func TestPipelineRunSortByStartTime(t *testing.T) {
 	noCompletionPR.Status.CompletionTime = nil
 
 	notStartedYet := tektontest.MakePRCompletion(clock, "notStarted", ns, success, nil, labels, 5)
-	noCompletionPR.Status.StartTime = nil
-	noCompletionPR.Status.CompletionTime = nil
+	notStartedYet.Status.StartTime = nil
+	notStartedYet.Status.CompletionTime = nil
+
+	prunsWithOneNotStarted := []tektonv1.PipelineRun{
+		*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, nil, labels, 30)),
+		*notStartedYet,
+	}
+
+	prunsWithJNotStarted := []tektonv1.PipelineRun{
+		*notStartedYet,
+		*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, nil, labels, 30)),
+	}
 
 	tests := []struct {
 		name     string
@@ -83,11 +153,31 @@ func TestPipelineRunSortByStartTime(t *testing.T) {
 		{
 			name: "not started yet",
 			pruns: []tektonv1.PipelineRun{
-				*notStartedYet,
 				*(tektontest.MakePRCompletion(clock, "otherFirst", ns, success, nil, labels, 30)),
 				*(tektontest.MakePRCompletion(clock, "otherSecond", ns, success, nil, labels, 10)),
+				*notStartedYet,
 			},
-			wantName: []string{"otherFirst", "otherSecond", "notStarted"},
+			wantName: []string{"notStarted", "otherFirst", "otherSecond"},
+		},
+		{
+			name:     "not started yet single",
+			pruns:    prunsWithOneNotStarted,
+			wantName: []string{"notStarted", "otherFirst"},
+		},
+		{
+			name:     "not started yet single j",
+			pruns:    prunsWithJNotStarted,
+			wantName: []string{"notStarted", "otherFirst"},
+		},
+		{
+			name:     "empty list",
+			pruns:    []tektonv1.PipelineRun{},
+			wantName: []string{},
+		},
+		{
+			name:     "single item",
+			pruns:    []tektonv1.PipelineRun{*startedEarlierPR},
+			wantName: []string{"earlier"},
 		},
 	}
 	for _, tt := range tests {