clean up

Author: wxiaoguang

models/git/commit_status.go

@@ -233,7 +233,7 @@ func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus {
 	var lastStatus *CommitStatus
 	state := api.CommitStatusSuccess
 	for _, status := range statuses {
-		if status.State.NoBetterThan(state) {
+		if status.State.HasHigherPriorityThan(state) {
 			state = status.State
 			lastStatus = status
 		}

modules/structs/commit_status.go

@@ -35,19 +35,10 @@ func (css CommitStatusState) String() string {
 	return string(css)
 }
 
-// NoBetterThan returns true if this State is no better than the given State
-// This function only handles the states defined in CommitStatusPriorities
-func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
-	// only handle the states with defined priorities above, it always returns false for undefined priorities
-	if _, exist := commitStatusPriorities[css]; !exist {
-		return false
-	}
-
-	if _, exist := commitStatusPriorities[css2]; !exist {
-		return false
-	}
-
-	return commitStatusPriorities[css] <= commitStatusPriorities[css2]
+// HasHigherPriorityThan returns true if this state has higher priority than the other
+// Undefined states are considered to have the highest priority like CommitStatusError(0)
+func (css CommitStatusState) HasHigherPriorityThan(other CommitStatusState) bool {
+	return commitStatusPriorities[css] < commitStatusPriorities[other]
 }
 
 // IsPending represents if commit status state is pending

modules/structs/commit_status_test.go

@@ -10,165 +10,21 @@ import (
 )
 
 func TestNoBetterThan(t *testing.T) {
-	type args struct {
-		css  CommitStatusState
-		css2 CommitStatusState
-	}
-	var unExpectedState CommitStatusState
 	tests := []struct {
-		name string
-		args args
-		want bool
+		s1, s2 CommitStatusState
+		higher bool
 	}{
-		{
-			name: "success is no better than success",
-			args: args{
-				css:  CommitStatusSuccess,
-				css2: CommitStatusSuccess,
-			},
-			want: true,
-		},
-		{
-			name: "success is no better than pending",
-			args: args{
-				css:  CommitStatusSuccess,
-				css2: CommitStatusPending,
-			},
-			want: false,
-		},
-		{
-			name: "success is no better than failure",
-			args: args{
-				css:  CommitStatusSuccess,
-				css2: CommitStatusFailure,
-			},
-			want: false,
-		},
-		{
-			name: "success is no better than error",
-			args: args{
-				css:  CommitStatusSuccess,
-				css2: CommitStatusError,
-			},
-			want: false,
-		},
-		{
-			name: "pending is no better than success",
-			args: args{
-				css:  CommitStatusPending,
-				css2: CommitStatusSuccess,
-			},
-			want: true,
-		},
-		{
-			name: "pending is no better than pending",
-			args: args{
-				css:  CommitStatusPending,
-				css2: CommitStatusPending,
-			},
-			want: true,
-		},
-		{
-			name: "pending is no better than failure",
-			args: args{
-				css:  CommitStatusPending,
-				css2: CommitStatusFailure,
-			},
-			want: false,
-		},
-		{
-			name: "pending is no better than error",
-			args: args{
-				css:  CommitStatusPending,
-				css2: CommitStatusError,
-			},
-			want: false,
-		},
-		{
-			name: "failure is no better than success",
-			args: args{
-				css:  CommitStatusFailure,
-				css2: CommitStatusSuccess,
-			},
-			want: true,
-		},
-		{
-			name: "failure is no better than pending",
-			args: args{
-				css:  CommitStatusFailure,
-				css2: CommitStatusPending,
-			},
-			want: true,
-		},
-		{
-			name: "failure is no better than failure",
-			args: args{
-				css:  CommitStatusFailure,
-				css2: CommitStatusFailure,
-			},
-			want: true,
-		},
-		{
-			name: "failure is no better than error",
-			args: args{
-				css:  CommitStatusFailure,
-				css2: CommitStatusError,
-			},
-			want: false,
-		},
-		{
-			name: "error is no better than success",
-			args: args{
-				css:  CommitStatusError,
-				css2: CommitStatusSuccess,
-			},
-			want: true,
-		},
-		{
-			name: "error is no better than pending",
-			args: args{
-				css:  CommitStatusError,
-				css2: CommitStatusPending,
-			},
-			want: true,
-		},
-		{
-			name: "error is no better than failure",
-			args: args{
-				css:  CommitStatusError,
-				css2: CommitStatusFailure,
-			},
-			want: true,
-		},
-		{
-			name: "error is no better than error",
-			args: args{
-				css:  CommitStatusError,
-				css2: CommitStatusError,
-			},
-			want: true,
-		},
-		{
-			name: "unExpectedState is no better than success",
-			args: args{
-				css:  unExpectedState,
-				css2: CommitStatusSuccess,
-			},
-			want: false,
-		},
-		{
-			name: "unExpectedState is no better than unExpectedState",
-			args: args{
-				css:  unExpectedState,
-				css2: unExpectedState,
-			},
-			want: false,
-		},
+		{CommitStatusError, CommitStatusFailure, true},
+		{CommitStatusFailure, CommitStatusWarning, true},
+		{CommitStatusWarning, CommitStatusPending, true},
+		{CommitStatusPending, CommitStatusSuccess, true},
+		{CommitStatusSuccess, CommitStatusSkipped, true},
+
+		{CommitStatusError, "unknown-xxx", false},
+		{"unknown-xxx", CommitStatusFailure, true},
 	}
 	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			result := tt.args.css.NoBetterThan(tt.args.css2)
-			assert.Equal(t, tt.want, result)
-		})
+		assert.Equal(t, tt.higher, tt.s1.HasHigherPriorityThan(tt.s2), "s1=%s, s2=%s, expected=%v", tt.s1, tt.s2, tt.higher)
 	}
+	assert.Equal(t, false, CommitStatusError.HasHigherPriorityThan(CommitStatusError))
 }

services/convert/status.go

@@ -43,12 +43,13 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r
 		TotalCount: len(statuses),
 		Repository: repo,
 		URL:        "",
+		State:      api.CommitStatusSuccess,
 	}
 
 	retStatus.Statuses = make([]*api.CommitStatus, 0, len(statuses))
 	for _, status := range statuses {
 		retStatus.Statuses = append(retStatus.Statuses, ToCommitStatus(ctx, status))
-		if retStatus.State == "" || status.State.NoBetterThan(retStatus.State) {
+		if status.State.HasHigherPriorityThan(retStatus.State) {
 			retStatus.State = status.State
 		}
 	}

services/pull/commit_status.go

@@ -46,13 +46,13 @@ func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus,
 
 			// If required rule not match any action, then it is pending
 			if targetStatus == "" {
-				if structs.CommitStatusPending.NoBetterThan(returnedStatus) {
+				if structs.CommitStatusPending.HasHigherPriorityThan(returnedStatus) {
 					returnedStatus = structs.CommitStatusPending
 				}
 				break
 			}
 
-			if targetStatus.NoBetterThan(returnedStatus) {
+			if targetStatus.HasHigherPriorityThan(returnedStatus) {
 				returnedStatus = targetStatus
 			}
 		}