refactor improve NoBetterThan (#26126)

- The `NoBetterThan` function can only handle comparisons between
"pending," "success," "error," and "failure." For any other comparison,
we directly return false. This prevents logic errors like the one in
#26121.
- The callers of the `NoBetterThan` function should also avoid making
incomparable calls.

---------

Co-authored-by: yp05327 <576951401@qq.com>
Co-authored-by: puni9869 <80308335+puni9869@users.noreply.github.com>
This commit is contained in:
caicandong 2023-07-26 16:52:07 +08:00 committed by GitHub
parent df9afe3aa8
commit 13359581df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 193 additions and 8 deletions

View file

@ -4,7 +4,7 @@
package structs
// CommitStatusState holds the state of a CommitStatus
// It can be "pending", "success", "error", "failure", and "warning"
// It can be "pending", "success", "error" and "failure"
type CommitStatusState string
const (
@ -18,14 +18,25 @@ const (
CommitStatusFailure CommitStatusState = "failure"
)
// NoBetterThan returns true if this State is no better than the given State
func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
commitStatusPriorities := map[CommitStatusState]int{
var commitStatusPriorities = map[CommitStatusState]int{
CommitStatusError: 0,
CommitStatusFailure: 1,
CommitStatusPending: 2,
CommitStatusSuccess: 3,
}
// 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 {
// NoBetterThan only handles the 4 states above
if _, exist := commitStatusPriorities[css]; !exist {
return false
}
if _, exist := commitStatusPriorities[css2]; !exist {
return false
}
return commitStatusPriorities[css] <= commitStatusPriorities[css2]
}

View file

@ -0,0 +1,174 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package structs
import (
"testing"
)
func TestNoBetterThan(t *testing.T) {
type args struct {
css CommitStatusState
css2 CommitStatusState
}
var unExpectedState CommitStatusState
tests := []struct {
name string
args args
want 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,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := tt.args.css.NoBetterThan(tt.args.css2)
if result != tt.want {
t.Errorf("NoBetterThan() = %v, want %v", result, tt.want)
}
})
}
}

View file

@ -48,7 +48,7 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r
retStatus.Statuses = make([]*api.CommitStatus, 0, len(statuses))
for _, status := range statuses {
retStatus.Statuses = append(retStatus.Statuses, ToCommitStatus(ctx, status))
if status.State.NoBetterThan(retStatus.State) {
if retStatus.State == "" || status.State.NoBetterThan(retStatus.State) {
retStatus.State = status.State
}
}

View file

@ -16514,7 +16514,7 @@
"x-go-package": "code.gitea.io/gitea/modules/structs"
},
"CommitStatusState": {
"description": "CommitStatusState holds the state of a CommitStatus\nIt can be \"pending\", \"success\", \"error\", \"failure\", and \"warning\"",
"description": "CommitStatusState holds the state of a CommitStatus\nIt can be \"pending\", \"success\", \"error\" and \"failure\"",
"type": "string",
"x-go-package": "code.gitea.io/gitea/modules/structs"
},