Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0cec4b8

Browse files
Fix actions skipped commit status indicator (#34507)
Addresses#34500Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parentc6e2093 commit0cec4b8

File tree

10 files changed

+112
-249
lines changed

10 files changed

+112
-249
lines changed

‎models/git/commit_status.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,18 +230,24 @@ func (status *CommitStatus) HideActionsURL(ctx context.Context) {
230230

231231
// CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc
232232
funcCalcCommitStatus(statuses []*CommitStatus)*CommitStatus {
233+
// This function is widely used, but it is not quite right.
234+
// Ideally it should return something like "CommitStatusSummary" with properly aggregated state.
235+
// GitHub's behavior: if all statuses are "skipped", GitHub will return "success" as the combined status.
233236
varlastStatus*CommitStatus
234237
state:=api.CommitStatusSuccess
235238
for_,status:=rangestatuses {
236-
ifstatus.State.NoBetterThan(state) {
239+
ifstate==status.State||status.State.HasHigherPriorityThan(state) {
237240
state=status.State
238241
lastStatus=status
239242
}
240243
}
241244
iflastStatus==nil {
242245
iflen(statuses)>0 {
246+
// FIXME: a bad case: Gitea just returns the first commit status, its status is "skipped" in this case.
243247
lastStatus=statuses[0]
244248
}else {
249+
// FIXME: another bad case: if the "statuses" slice is empty, the returned value is an invalid CommitStatus, all its fields are empty.
250+
// Frontend code (tmpl&vue) sometimes depend on the empty fields to skip rendering commit status elements (need to double check in the future)
245251
lastStatus=&CommitStatus{}
246252
}
247253
}

‎models/git/commit_status_summary.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,11 @@ func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) er
5959
iferr!=nil {
6060
returnerr
6161
}
62-
state:=CalcCommitStatus(commitStatuses)
62+
// it guarantees that commitStatuses is not empty because this function is always called after a commit status is created
63+
iflen(commitStatuses)==0 {
64+
setting.PanicInDevOrTesting("no commit statuses found for repo %d and sha %s",repoID,sha)
65+
}
66+
state:=CalcCommitStatus(commitStatuses)// non-empty commitStatuses is guaranteed
6367
// mysql will return 0 when update a record which state hasn't been changed which behaviour is different from other database,
6468
// so we need to use insert in on duplicate
6569
ifsetting.Database.Type.IsMySQL() {

‎modules/structs/commit_status.go‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ const (
1818
CommitStatusFailureCommitStatusState="failure"
1919
// CommitStatusWarning is for when the CommitStatus is Warning
2020
CommitStatusWarningCommitStatusState="warning"
21+
// CommitStatusSkipped is for when CommitStatus is Skipped
22+
CommitStatusSkippedCommitStatusState="skipped"
2123
)
2224

2325
varcommitStatusPriorities=map[CommitStatusState]int{
@@ -26,25 +28,17 @@ var commitStatusPriorities = map[CommitStatusState]int{
2628
CommitStatusWarning:2,
2729
CommitStatusPending:3,
2830
CommitStatusSuccess:4,
31+
CommitStatusSkipped:5,
2932
}
3033

3134
func (cssCommitStatusState)String()string {
3235
returnstring(css)
3336
}
3437

35-
// NoBetterThan returns true if this State is no better than the given State
36-
// This function only handles the states defined in CommitStatusPriorities
37-
func (cssCommitStatusState)NoBetterThan(css2CommitStatusState)bool {
38-
// NoBetterThan only handles the 5 states above
39-
if_,exist:=commitStatusPriorities[css];!exist {
40-
returnfalse
41-
}
42-
43-
if_,exist:=commitStatusPriorities[css2];!exist {
44-
returnfalse
45-
}
46-
47-
returncommitStatusPriorities[css]<=commitStatusPriorities[css2]
38+
// HasHigherPriorityThan returns true if this state has higher priority than the other
39+
// Undefined states are considered to have the highest priority like CommitStatusError(0)
40+
func (cssCommitStatusState)HasHigherPriorityThan(otherCommitStatusState)bool {
41+
returncommitStatusPriorities[css]<commitStatusPriorities[other]
4842
}
4943

5044
// IsPending represents if commit status state is pending

‎modules/structs/commit_status_test.go‎

Lines changed: 12 additions & 156 deletions
Original file line numberDiff line numberDiff line change
@@ -10,165 +10,21 @@ import (
1010
)
1111

1212
funcTestNoBetterThan(t*testing.T) {
13-
typeargsstruct {
14-
cssCommitStatusState
15-
css2CommitStatusState
16-
}
17-
varunExpectedStateCommitStatusState
1813
tests:= []struct {
19-
namestring
20-
argsargs
21-
wantbool
14+
s1,s2CommitStatusState
15+
higherbool
2216
}{
23-
{
24-
name:"success is no better than success",
25-
args:args{
26-
css:CommitStatusSuccess,
27-
css2:CommitStatusSuccess,
28-
},
29-
want:true,
30-
},
31-
{
32-
name:"success is no better than pending",
33-
args:args{
34-
css:CommitStatusSuccess,
35-
css2:CommitStatusPending,
36-
},
37-
want:false,
38-
},
39-
{
40-
name:"success is no better than failure",
41-
args:args{
42-
css:CommitStatusSuccess,
43-
css2:CommitStatusFailure,
44-
},
45-
want:false,
46-
},
47-
{
48-
name:"success is no better than error",
49-
args:args{
50-
css:CommitStatusSuccess,
51-
css2:CommitStatusError,
52-
},
53-
want:false,
54-
},
55-
{
56-
name:"pending is no better than success",
57-
args:args{
58-
css:CommitStatusPending,
59-
css2:CommitStatusSuccess,
60-
},
61-
want:true,
62-
},
63-
{
64-
name:"pending is no better than pending",
65-
args:args{
66-
css:CommitStatusPending,
67-
css2:CommitStatusPending,
68-
},
69-
want:true,
70-
},
71-
{
72-
name:"pending is no better than failure",
73-
args:args{
74-
css:CommitStatusPending,
75-
css2:CommitStatusFailure,
76-
},
77-
want:false,
78-
},
79-
{
80-
name:"pending is no better than error",
81-
args:args{
82-
css:CommitStatusPending,
83-
css2:CommitStatusError,
84-
},
85-
want:false,
86-
},
87-
{
88-
name:"failure is no better than success",
89-
args:args{
90-
css:CommitStatusFailure,
91-
css2:CommitStatusSuccess,
92-
},
93-
want:true,
94-
},
95-
{
96-
name:"failure is no better than pending",
97-
args:args{
98-
css:CommitStatusFailure,
99-
css2:CommitStatusPending,
100-
},
101-
want:true,
102-
},
103-
{
104-
name:"failure is no better than failure",
105-
args:args{
106-
css:CommitStatusFailure,
107-
css2:CommitStatusFailure,
108-
},
109-
want:true,
110-
},
111-
{
112-
name:"failure is no better than error",
113-
args:args{
114-
css:CommitStatusFailure,
115-
css2:CommitStatusError,
116-
},
117-
want:false,
118-
},
119-
{
120-
name:"error is no better than success",
121-
args:args{
122-
css:CommitStatusError,
123-
css2:CommitStatusSuccess,
124-
},
125-
want:true,
126-
},
127-
{
128-
name:"error is no better than pending",
129-
args:args{
130-
css:CommitStatusError,
131-
css2:CommitStatusPending,
132-
},
133-
want:true,
134-
},
135-
{
136-
name:"error is no better than failure",
137-
args:args{
138-
css:CommitStatusError,
139-
css2:CommitStatusFailure,
140-
},
141-
want:true,
142-
},
143-
{
144-
name:"error is no better than error",
145-
args:args{
146-
css:CommitStatusError,
147-
css2:CommitStatusError,
148-
},
149-
want:true,
150-
},
151-
{
152-
name:"unExpectedState is no better than success",
153-
args:args{
154-
css:unExpectedState,
155-
css2:CommitStatusSuccess,
156-
},
157-
want:false,
158-
},
159-
{
160-
name:"unExpectedState is no better than unExpectedState",
161-
args:args{
162-
css:unExpectedState,
163-
css2:unExpectedState,
164-
},
165-
want:false,
166-
},
17+
{CommitStatusError,CommitStatusFailure,true},
18+
{CommitStatusFailure,CommitStatusWarning,true},
19+
{CommitStatusWarning,CommitStatusPending,true},
20+
{CommitStatusPending,CommitStatusSuccess,true},
21+
{CommitStatusSuccess,CommitStatusSkipped,true},
22+
23+
{CommitStatusError,"unknown-xxx",false},
24+
{"unknown-xxx",CommitStatusFailure,true},
16725
}
16826
for_,tt:=rangetests {
169-
t.Run(tt.name,func(t*testing.T) {
170-
result:=tt.args.css.NoBetterThan(tt.args.css2)
171-
assert.Equal(t,tt.want,result)
172-
})
27+
assert.Equal(t,tt.higher,tt.s1.HasHigherPriorityThan(tt.s2),"s1=%s, s2=%s, expected=%v",tt.s1,tt.s2,tt.higher)
17328
}
29+
assert.False(t,CommitStatusError.HasHigherPriorityThan(CommitStatusError))
17430
}

‎services/actions/commit_status.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,14 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
149149

150150
functoCommitStatus(status actions_model.Status) api.CommitStatusState {
151151
switchstatus {
152-
caseactions_model.StatusSuccess,actions_model.StatusSkipped:
152+
caseactions_model.StatusSuccess:
153153
returnapi.CommitStatusSuccess
154154
caseactions_model.StatusFailure,actions_model.StatusCancelled:
155155
returnapi.CommitStatusFailure
156156
caseactions_model.StatusWaiting,actions_model.StatusBlocked,actions_model.StatusRunning:
157157
returnapi.CommitStatusPending
158+
caseactions_model.StatusSkipped:
159+
returnapi.CommitStatusSkipped
158160
default:
159161
returnapi.CommitStatusError
160162
}

‎services/convert/status.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,14 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r
4242
SHA:statuses[0].SHA,
4343
TotalCount:len(statuses),
4444
Repository:repo,
45-
URL:"",
45+
URL:"",// never set or used?
46+
State:api.CommitStatusSuccess,
4647
}
4748

4849
retStatus.Statuses=make([]*api.CommitStatus,0,len(statuses))
4950
for_,status:=rangestatuses {
5051
retStatus.Statuses=append(retStatus.Statuses,ToCommitStatus(ctx,status))
51-
ifretStatus.State==""||status.State.NoBetterThan(retStatus.State) {
52+
ifstatus.State.HasHigherPriorityThan(retStatus.State) {
5253
retStatus.State=status.State
5354
}
5455
}
@@ -57,9 +58,13 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r
5758
// > failure if any of the contexts report as error or failure
5859
// > pending if there are no statuses or a context is pending
5960
// > success if the latest status for all contexts is success
60-
ifretStatus.State.IsError() {
61-
retStatus.State=api.CommitStatusFailure
61+
switchretStatus.State {
62+
caseapi.CommitStatusSkipped:
63+
retStatus.State=api.CommitStatusSuccess// all skipped means success
64+
caseapi.CommitStatusPending,api.CommitStatusSuccess:
65+
// use the current state for pending or success
66+
default:
67+
retStatus.State=api.CommitStatusFailure// otherwise, it is a failure
6268
}
63-
6469
returnretStatus
6570
}

‎services/pull/commit_status.go‎

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -46,57 +46,31 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus,
4646

4747
// If required rule not match any action, then it is pending
4848
iftargetStatus=="" {
49-
ifstructs.CommitStatusPending.NoBetterThan(returnedStatus) {
49+
ifstructs.CommitStatusPending.HasHigherPriorityThan(returnedStatus) {
5050
returnedStatus=structs.CommitStatusPending
5151
}
5252
break
5353
}
5454

55-
iftargetStatus.NoBetterThan(returnedStatus) {
55+
iftargetStatus.HasHigherPriorityThan(returnedStatus) {
5656
returnedStatus=targetStatus
5757
}
5858
}
5959
}
6060

6161
ifmatchedCount==0&&returnedStatus==structs.CommitStatusSuccess {
62-
status:=git_model.CalcCommitStatus(commitStatuses)
63-
ifstatus!=nil {
64-
returnstatus.State
62+
iflen(commitStatuses)==0 {
63+
// "no statuses" should mean "pending"
64+
returnstructs.CommitStatusPending
6565
}
66-
returnstructs.CommitStatusSuccess
67-
}
68-
69-
returnreturnedStatus
70-
}
71-
72-
// IsCommitStatusContextSuccess returns true if all required status check contexts succeed.
73-
funcIsCommitStatusContextSuccess(commitStatuses []*git_model.CommitStatus,requiredContexts []string)bool {
74-
// If no specific context is required, require that last commit status is a success
75-
iflen(requiredContexts)==0 {
7666
status:=git_model.CalcCommitStatus(commitStatuses)
77-
ifstatus==nil||status.State!=structs.CommitStatusSuccess {
78-
returnfalse
67+
ifstatus.State==structs.CommitStatusSkipped {
68+
returnstructs.CommitStatusSuccess// if all statuses are skipped, return success
7969
}
80-
returntrue
70+
returnstatus.State
8171
}
8272

83-
for_,ctx:=rangerequiredContexts {
84-
varfoundbool
85-
for_,commitStatus:=rangecommitStatuses {
86-
ifcommitStatus.Context==ctx {
87-
ifcommitStatus.State!=structs.CommitStatusSuccess {
88-
returnfalse
89-
}
90-
91-
found=true
92-
break
93-
}
94-
}
95-
if!found {
96-
returnfalse
97-
}
98-
}
99-
returntrue
73+
returnreturnedStatus
10074
}
10175

10276
// IsPullCommitStatusPass returns if all required status checks PASS

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp