Improve job status check handling (#28)

* improve job status check handling Signed-off-by: hlts2 <[email protected]> * Update internal/validators/status/validator_test.go * apply suggestion Signed-off-by: hlts2 <[email protected]> * Update internal/validators/status/validator_test.go Co-authored-by: Ryota <[email protected]> * Update internal/validators/status/validator_test.go Co-authored-by: Ryota <[email protected]> * fix comment Signed-off-by: hlts2 <[email protected]> Co-authored-by: Ryota <[email protected]>
upsidr · Apr 13, 2022 · 9898273 · 9898273
1 parent 94e9047
commit 9898273
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 0 deletions.
diff --git a/internal/validators/status/validator.go b/internal/validators/status/validator.go
@@ -135,11 +135,20 @@ func (sv *statusValidator) listGhaStatuses(ctx context.Context) ([]*ghaStatus, e
 		return nil, err
 	}
 
+	// Because multiple jobs with the same name may exist when jobs are created dynamically by third-party tools, etc.,
+	// only the latest job should be managed.
+	currentJobs := make(map[string]struct{})
+
 	ghaStatuses := make([]*ghaStatus, 0, len(combined.Statuses))
 	for _, s := range combined.Statuses {
 		if s.Context == nil || s.State == nil {
 			return nil, fmt.Errorf("%w context: %v, status: %v", ErrInvalidCombinedStatusResponse, s.Context, s.State)
 		}
+		if _, ok := currentJobs[*s.Context]; ok {
+			continue
+		}
+		currentJobs[*s.Context] = struct{}{}
+
 		ghaStatuses = append(ghaStatuses, &ghaStatus{
 			Job:   *s.Context,
 			State: *s.State,
@@ -155,6 +164,11 @@ func (sv *statusValidator) listGhaStatuses(ctx context.Context) ([]*ghaStatus, e
 		if run.Name == nil || run.Status == nil {
 			return nil, fmt.Errorf("%w name: %v, status: %v", ErrInvalidCheckRunResponse, run.Name, run.Status)
 		}
+		if _, ok := currentJobs[*run.Name]; ok {
+			continue
+		}
+		currentJobs[*run.Name] = struct{}{}
+
 		ghaStatus := &ghaStatus{
 			Job: *run.Name,
 		}

diff --git a/internal/validators/status/validator_test.go b/internal/validators/status/validator_test.go
@@ -317,6 +317,93 @@ func Test_statusValidator_listStatues(t *testing.T) {
 		want    []*ghaStatus
 	}
 	tests := map[string]test{
+		"succeeds to get job statuses even if the same job exists": func() test {
+			c := &mock.Client{
+				GetCombinedStatusFunc: func(ctx context.Context, owner, repo, ref string, opts *github.ListOptions) (*github.CombinedStatus, *github.Response, error) {
+					return &github.CombinedStatus{
+						Statuses: []*github.RepoStatus{
+							// The first element here is the latest state.
+							{
+								Context: stringPtr("job-01"),
+								State:   stringPtr(successState),
+							},
+							{
+								Context: stringPtr("job-01"), // Same as above job name, and thus should be disregarded as old job status.
+								State:   stringPtr(errorState),
+							},
+						},
+					}, nil, nil
+				},
+				ListCheckRunsForRefFunc: func(ctx context.Context, owner, repo, ref string, opts *github.ListCheckRunsOptions) (*github.ListCheckRunsResults, *github.Response, error) {
+					return &github.ListCheckRunsResults{
+						CheckRuns: []*github.CheckRun{
+							// The first element here is the latest state.
+							{
+								Name:   stringPtr("job-02"),
+								Status: stringPtr("failure"),
+							},
+							{
+								Name:       stringPtr("job-02"), // Same as above job name, and thus should be disregarded as old job status.
+								Status:     stringPtr(checkRunCompletedStatus),
+								Conclusion: stringPtr(checkRunNeutralConclusion),
+							},
+							{
+								Name:       stringPtr("job-03"),
+								Status:     stringPtr(checkRunCompletedStatus),
+								Conclusion: stringPtr(checkRunNeutralConclusion),
+							},
+							{
+								Name:       stringPtr("job-04"),
+								Status:     stringPtr(checkRunCompletedStatus),
+								Conclusion: stringPtr(checkRunSuccessConclusion),
+							},
+							{
+								Name:       stringPtr("job-05"),
+								Status:     stringPtr(checkRunCompletedStatus),
+								Conclusion: stringPtr("failure"),
+							},
+							{
+								Name:       stringPtr("job-06"),
+								Status:     stringPtr(checkRunCompletedStatus),
+								Conclusion: stringPtr(checkRunSkipConclusion),
+							},
+						},
+					}, nil, nil
+				},
+			}
+			return test{
+				fields: fields{
+					client:      c,
+					selfJobName: "self-job",
+					owner:       "test-owner",
+					repo:        "test-repo",
+					ref:         "main",
+				},
+				wantErr: false,
+				want: []*ghaStatus{
+					{
+						Job:   "job-01",
+						State: successState,
+					},
+					{
+						Job:   "job-02",
+						State: pendingState,
+					},
+					{
+						Job:   "job-03",
+						State: successState,
+					},
+					{
+						Job:   "job-04",
+						State: successState,
+					},
+					{
+						Job:   "job-05",
+						State: errorState,
+					},
+				},
+			}
+		}(),
 		"returns error when the GetCombinedStatus returns an error": func() test {
 			c := &mock.Client{
 				GetCombinedStatusFunc: func(ctx context.Context, owner, repo, ref string, opts *github.ListOptions) (*github.CombinedStatus, *github.Response, error) {