Skip to content

Commit 9898273

Browse files
hlts2rytswd
andauthored
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]>
1 parent 94e9047 commit 9898273

File tree

2 files changed

+101
-0
lines changed

2 files changed

+101
-0
lines changed

internal/validators/status/validator.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,20 @@ func (sv *statusValidator) listGhaStatuses(ctx context.Context) ([]*ghaStatus, e
135135
return nil, err
136136
}
137137

138+
// Because multiple jobs with the same name may exist when jobs are created dynamically by third-party tools, etc.,
139+
// only the latest job should be managed.
140+
currentJobs := make(map[string]struct{})
141+
138142
ghaStatuses := make([]*ghaStatus, 0, len(combined.Statuses))
139143
for _, s := range combined.Statuses {
140144
if s.Context == nil || s.State == nil {
141145
return nil, fmt.Errorf("%w context: %v, status: %v", ErrInvalidCombinedStatusResponse, s.Context, s.State)
142146
}
147+
if _, ok := currentJobs[*s.Context]; ok {
148+
continue
149+
}
150+
currentJobs[*s.Context] = struct{}{}
151+
143152
ghaStatuses = append(ghaStatuses, &ghaStatus{
144153
Job: *s.Context,
145154
State: *s.State,
@@ -155,6 +164,11 @@ func (sv *statusValidator) listGhaStatuses(ctx context.Context) ([]*ghaStatus, e
155164
if run.Name == nil || run.Status == nil {
156165
return nil, fmt.Errorf("%w name: %v, status: %v", ErrInvalidCheckRunResponse, run.Name, run.Status)
157166
}
167+
if _, ok := currentJobs[*run.Name]; ok {
168+
continue
169+
}
170+
currentJobs[*run.Name] = struct{}{}
171+
158172
ghaStatus := &ghaStatus{
159173
Job: *run.Name,
160174
}

internal/validators/status/validator_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,93 @@ func Test_statusValidator_listStatues(t *testing.T) {
317317
want []*ghaStatus
318318
}
319319
tests := map[string]test{
320+
"succeeds to get job statuses even if the same job exists": func() test {
321+
c := &mock.Client{
322+
GetCombinedStatusFunc: func(ctx context.Context, owner, repo, ref string, opts *github.ListOptions) (*github.CombinedStatus, *github.Response, error) {
323+
return &github.CombinedStatus{
324+
Statuses: []*github.RepoStatus{
325+
// The first element here is the latest state.
326+
{
327+
Context: stringPtr("job-01"),
328+
State: stringPtr(successState),
329+
},
330+
{
331+
Context: stringPtr("job-01"), // Same as above job name, and thus should be disregarded as old job status.
332+
State: stringPtr(errorState),
333+
},
334+
},
335+
}, nil, nil
336+
},
337+
ListCheckRunsForRefFunc: func(ctx context.Context, owner, repo, ref string, opts *github.ListCheckRunsOptions) (*github.ListCheckRunsResults, *github.Response, error) {
338+
return &github.ListCheckRunsResults{
339+
CheckRuns: []*github.CheckRun{
340+
// The first element here is the latest state.
341+
{
342+
Name: stringPtr("job-02"),
343+
Status: stringPtr("failure"),
344+
},
345+
{
346+
Name: stringPtr("job-02"), // Same as above job name, and thus should be disregarded as old job status.
347+
Status: stringPtr(checkRunCompletedStatus),
348+
Conclusion: stringPtr(checkRunNeutralConclusion),
349+
},
350+
{
351+
Name: stringPtr("job-03"),
352+
Status: stringPtr(checkRunCompletedStatus),
353+
Conclusion: stringPtr(checkRunNeutralConclusion),
354+
},
355+
{
356+
Name: stringPtr("job-04"),
357+
Status: stringPtr(checkRunCompletedStatus),
358+
Conclusion: stringPtr(checkRunSuccessConclusion),
359+
},
360+
{
361+
Name: stringPtr("job-05"),
362+
Status: stringPtr(checkRunCompletedStatus),
363+
Conclusion: stringPtr("failure"),
364+
},
365+
{
366+
Name: stringPtr("job-06"),
367+
Status: stringPtr(checkRunCompletedStatus),
368+
Conclusion: stringPtr(checkRunSkipConclusion),
369+
},
370+
},
371+
}, nil, nil
372+
},
373+
}
374+
return test{
375+
fields: fields{
376+
client: c,
377+
selfJobName: "self-job",
378+
owner: "test-owner",
379+
repo: "test-repo",
380+
ref: "main",
381+
},
382+
wantErr: false,
383+
want: []*ghaStatus{
384+
{
385+
Job: "job-01",
386+
State: successState,
387+
},
388+
{
389+
Job: "job-02",
390+
State: pendingState,
391+
},
392+
{
393+
Job: "job-03",
394+
State: successState,
395+
},
396+
{
397+
Job: "job-04",
398+
State: successState,
399+
},
400+
{
401+
Job: "job-05",
402+
State: errorState,
403+
},
404+
},
405+
}
406+
}(),
320407
"returns error when the GetCombinedStatus returns an error": func() test {
321408
c := &mock.Client{
322409
GetCombinedStatusFunc: func(ctx context.Context, owner, repo, ref string, opts *github.ListOptions) (*github.CombinedStatus, *github.Response, error) {

0 commit comments

Comments
 (0)