Add unit tests for DependsOn

kubernetes-sigs · Jan 2, 2025 · 22a408b · 22a408b
1 parent 34c0593
commit 22a408b
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 8 deletions.
diff --git a/pkg/controllers/depends_on.go b/pkg/controllers/depends_on.go
@@ -4,22 +4,21 @@ import (
 	jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2"
 )
 
-// isJobReachedStatus checks if ReplicatedJob reaches Ready or Complete status.
-func isJobReachedStatus(dependsOnJobName string, dependsOnJobStatus jobset.DependsOnStatus, rJobsReplicas map[string]int32, rJobsStatuses []jobset.ReplicatedJobStatus) bool {
-
+// isDependsOnJobReachedStatus checks if depends on ReplicatedJob reaches Ready or Complete status.
+func isDependsOnJobReachedStatus(dependsOnJob jobset.DependsOn, dependsOnJobReplicas int32, rJobsStatuses []jobset.ReplicatedJobStatus) bool {
 	// If the actual status is empty, return false.
-	actualStatus := findReplicatedJobStatus(rJobsStatuses, dependsOnJobName)
+	actualStatus := findReplicatedJobStatus(rJobsStatuses, dependsOnJob.Name)
 	if actualStatus == nil {
 		return false
 	}
 
 	// For Complete status, number of replicas must be equal to number of succeeded Jobs.
-	if dependsOnJobStatus == jobset.CompleteStatus && rJobsReplicas[dependsOnJobName] == actualStatus.Succeeded {
+	if dependsOnJob.Status == jobset.CompleteStatus && dependsOnJobReplicas == actualStatus.Succeeded {
 		return true
 	}
 
 	// For Ready status, number of replicas must be equal to sum of ready, failed, and succeeded Jobs.
-	if dependsOnJobStatus == jobset.ReadyStatus && rJobsReplicas[dependsOnJobName] == actualStatus.Failed+actualStatus.Ready+actualStatus.Succeeded {
+	if dependsOnJob.Status == jobset.ReadyStatus && dependsOnJobReplicas == actualStatus.Failed+actualStatus.Ready+actualStatus.Succeeded {
 		return true
 	}
 

diff --git a/pkg/controllers/depends_on_test.go b/pkg/controllers/depends_on_test.go
@@ -0,0 +1,117 @@
+package controllers
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2"
+)
+
+func TestIsDependsOnJobReachedStatus(t *testing.T) {
+	tests := []struct {
+		name                 string
+		dependsOnJob         jobset.DependsOn
+		dependsOnJobReplicas int32
+		rJobsStatuses        []jobset.ReplicatedJobStatus
+		expected             bool
+	}{
+		{
+			name: "status for ReplicatedJob is nil",
+			dependsOnJob: jobset.DependsOn{
+				Name: "initializer", Status: jobset.CompleteStatus,
+			},
+			dependsOnJobReplicas: 1,
+			rJobsStatuses: []jobset.ReplicatedJobStatus{
+				{
+					Name:      "invalid",
+					Ready:     0,
+					Succeeded: 1,
+					Failed:    0,
+					Suspended: 0,
+					Active:    0,
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "depends on ReplicatedJob reaches complete status",
+			dependsOnJob: jobset.DependsOn{
+				Name: "initializer", Status: jobset.CompleteStatus,
+			},
+			dependsOnJobReplicas: 2,
+			rJobsStatuses: []jobset.ReplicatedJobStatus{
+				{
+					Name:      "initializer",
+					Ready:     0,
+					Succeeded: 2,
+					Failed:    0,
+					Suspended: 0,
+					Active:    0,
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "depends on ReplicatedJob doesn't reach complete status",
+			dependsOnJob: jobset.DependsOn{
+				Name: "initializer", Status: jobset.CompleteStatus,
+			},
+			dependsOnJobReplicas: 2,
+			rJobsStatuses: []jobset.ReplicatedJobStatus{
+				{
+					Name:      "initializer",
+					Ready:     1,
+					Succeeded: 1,
+					Failed:    0,
+					Suspended: 0,
+					Active:    0,
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "depends on ReplicatedJob reaches ready status",
+			dependsOnJob: jobset.DependsOn{
+				Name: "initializer", Status: jobset.ReadyStatus,
+			},
+			dependsOnJobReplicas: 3,
+			rJobsStatuses: []jobset.ReplicatedJobStatus{
+				{
+					Name:      "initializer",
+					Ready:     1,
+					Succeeded: 1,
+					Failed:    1,
+					Suspended: 0,
+					Active:    0,
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "depends on ReplicatedJob doesn't reach ready status",
+			dependsOnJob: jobset.DependsOn{
+				Name: "initializer", Status: jobset.ReadyStatus,
+			},
+			dependsOnJobReplicas: 3,
+			rJobsStatuses: []jobset.ReplicatedJobStatus{
+				{
+					Name:      "initializer",
+					Ready:     2,
+					Succeeded: 0,
+					Failed:    0,
+					Suspended: 0,
+					Active:    1,
+				},
+			},
+			expected: false,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := isDependsOnJobReachedStatus(tc.dependsOnJob, tc.dependsOnJobReplicas, tc.rJobsStatuses)
+			if diff := cmp.Diff(tc.expected, actual); diff != "" {
+				t.Errorf("unexpected finished value (+got/-want): %s", diff)
+			}
+		})
+	}
+}
diff --git a/pkg/controllers/jobset_controller.go b/pkg/controllers/jobset_controller.go
@@ -424,7 +424,7 @@ func (r *JobSetReconciler) resumeJobsIfNecessary(ctx context.Context, js *jobset
 		rJobsReplicas[replicatedJob.Name] = replicatedJob.Replicas
 
 		// For depends on, the Job is created only after the previous replicatedJob reached the status.
-		if replicatedJob.DependsOn != nil && !isJobReachedStatus(replicatedJob.DependsOn[0].Name, replicatedJob.DependsOn[0].Status, rJobsReplicas, replicatedJobStatuses) {
+		if replicatedJob.DependsOn != nil && !isDependsOnJobReachedStatus(replicatedJob.DependsOn[0], rJobsReplicas[replicatedJob.DependsOn[0].Name], replicatedJobStatuses) {
 			continue
 		}
 
@@ -518,7 +518,7 @@ func (r *JobSetReconciler) reconcileReplicatedJobs(ctx context.Context, js *jobs
 		rJobsReplicas[replicatedJob.Name] = replicatedJob.Replicas
 
 		// For depends on, the Job is created only after the previous replicatedJob reached the status.
-		if replicatedJob.DependsOn != nil && !isJobReachedStatus(replicatedJob.DependsOn[0].Name, replicatedJob.DependsOn[0].Status, rJobsReplicas, replicatedJobStatuses) {
+		if replicatedJob.DependsOn != nil && !isDependsOnJobReachedStatus(replicatedJob.DependsOn[0], rJobsReplicas[replicatedJob.DependsOn[0].Name], replicatedJobStatuses) {
 			continue
 		}