aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--models/git/commit_status.go8
-rw-r--r--models/git/commit_status_summary.go6
-rw-r--r--modules/structs/commit_status.go20
-rw-r--r--modules/structs/commit_status_test.go168
-rw-r--r--services/actions/commit_status.go4
-rw-r--r--services/convert/status.go15
-rw-r--r--services/pull/commit_status.go44
-rw-r--r--services/pull/commit_status_test.go90
-rw-r--r--templates/repo/commit_status.tmpl3
-rw-r--r--web_src/js/components/DashboardRepoList.vue3
10 files changed, 112 insertions, 249 deletions
diff --git a/models/git/commit_status.go b/models/git/commit_status.go
index 0e4e8215b0..2e765391b8 100644
--- a/models/git/commit_status.go
+++ b/models/git/commit_status.go
@@ -230,18 +230,24 @@ func (status *CommitStatus) HideActionsURL(ctx context.Context) {
// CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc
func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus {
+ // This function is widely used, but it is not quite right.
+ // Ideally it should return something like "CommitStatusSummary" with properly aggregated state.
+ // GitHub's behavior: if all statuses are "skipped", GitHub will return "success" as the combined status.
var lastStatus *CommitStatus
state := api.CommitStatusSuccess
for _, status := range statuses {
- if status.State.NoBetterThan(state) {
+ if state == status.State || status.State.HasHigherPriorityThan(state) {
state = status.State
lastStatus = status
}
}
if lastStatus == nil {
if len(statuses) > 0 {
+ // FIXME: a bad case: Gitea just returns the first commit status, its status is "skipped" in this case.
lastStatus = statuses[0]
} else {
+ // FIXME: another bad case: if the "statuses" slice is empty, the returned value is an invalid CommitStatus, all its fields are empty.
+ // Frontend code (tmpl&vue) sometimes depend on the empty fields to skip rendering commit status elements (need to double check in the future)
lastStatus = &CommitStatus{}
}
}
diff --git a/models/git/commit_status_summary.go b/models/git/commit_status_summary.go
index a3f440fd86..774e49bb98 100644
--- a/models/git/commit_status_summary.go
+++ b/models/git/commit_status_summary.go
@@ -59,7 +59,11 @@ func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) er
if err != nil {
return err
}
- state := CalcCommitStatus(commitStatuses)
+ // it guarantees that commitStatuses is not empty because this function is always called after a commit status is created
+ if len(commitStatuses) == 0 {
+ setting.PanicInDevOrTesting("no commit statuses found for repo %d and sha %s", repoID, sha)
+ }
+ state := CalcCommitStatus(commitStatuses) // non-empty commitStatuses is guaranteed
// mysql will return 0 when update a record which state hasn't been changed which behaviour is different from other database,
// so we need to use insert in on duplicate
if setting.Database.Type.IsMySQL() {
diff --git a/modules/structs/commit_status.go b/modules/structs/commit_status.go
index dc880ef5eb..398001974d 100644
--- a/modules/structs/commit_status.go
+++ b/modules/structs/commit_status.go
@@ -18,6 +18,8 @@ const (
CommitStatusFailure CommitStatusState = "failure"
// CommitStatusWarning is for when the CommitStatus is Warning
CommitStatusWarning CommitStatusState = "warning"
+ // CommitStatusSkipped is for when CommitStatus is Skipped
+ CommitStatusSkipped CommitStatusState = "skipped"
)
var commitStatusPriorities = map[CommitStatusState]int{
@@ -26,25 +28,17 @@ var commitStatusPriorities = map[CommitStatusState]int{
CommitStatusWarning: 2,
CommitStatusPending: 3,
CommitStatusSuccess: 4,
+ CommitStatusSkipped: 5,
}
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 {
- // NoBetterThan only handles the 5 states above
- 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
diff --git a/modules/structs/commit_status_test.go b/modules/structs/commit_status_test.go
index 88e09aadc1..c11daf248d 100644
--- a/modules/structs/commit_status_test.go
+++ b/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.False(t, CommitStatusError.HasHigherPriorityThan(CommitStatusError))
}
diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go
index 4541b3af4c..5d17df8047 100644
--- a/services/actions/commit_status.go
+++ b/services/actions/commit_status.go
@@ -149,12 +149,14 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er
func toCommitStatus(status actions_model.Status) api.CommitStatusState {
switch status {
- case actions_model.StatusSuccess, actions_model.StatusSkipped:
+ case actions_model.StatusSuccess:
return api.CommitStatusSuccess
case actions_model.StatusFailure, actions_model.StatusCancelled:
return api.CommitStatusFailure
case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning:
return api.CommitStatusPending
+ case actions_model.StatusSkipped:
+ return api.CommitStatusSkipped
default:
return api.CommitStatusError
}
diff --git a/services/convert/status.go b/services/convert/status.go
index 6cef63c1cd..4fffbfbe5e 100644
--- a/services/convert/status.go
+++ b/services/convert/status.go
@@ -42,13 +42,14 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r
SHA: statuses[0].SHA,
TotalCount: len(statuses),
Repository: repo,
- URL: "",
+ URL: "", // never set or used?
+ 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
}
}
@@ -57,9 +58,13 @@ func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, r
// > failure if any of the contexts report as error or failure
// > pending if there are no statuses or a context is pending
// > success if the latest status for all contexts is success
- if retStatus.State.IsError() {
- retStatus.State = api.CommitStatusFailure
+ switch retStatus.State {
+ case api.CommitStatusSkipped:
+ retStatus.State = api.CommitStatusSuccess // all skipped means success
+ case api.CommitStatusPending, api.CommitStatusSuccess:
+ // use the current state for pending or success
+ default:
+ retStatus.State = api.CommitStatusFailure // otherwise, it is a failure
}
-
return retStatus
}
diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go
index 553496713f..58d26c5a00 100644
--- a/services/pull/commit_status.go
+++ b/services/pull/commit_status.go
@@ -46,57 +46,31 @@ 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
}
}
}
if matchedCount == 0 && returnedStatus == structs.CommitStatusSuccess {
- status := git_model.CalcCommitStatus(commitStatuses)
- if status != nil {
- return status.State
+ if len(commitStatuses) == 0 {
+ // "no statuses" should mean "pending"
+ return structs.CommitStatusPending
}
- return structs.CommitStatusSuccess
- }
-
- return returnedStatus
-}
-
-// IsCommitStatusContextSuccess returns true if all required status check contexts succeed.
-func IsCommitStatusContextSuccess(commitStatuses []*git_model.CommitStatus, requiredContexts []string) bool {
- // If no specific context is required, require that last commit status is a success
- if len(requiredContexts) == 0 {
status := git_model.CalcCommitStatus(commitStatuses)
- if status == nil || status.State != structs.CommitStatusSuccess {
- return false
+ if status.State == structs.CommitStatusSkipped {
+ return structs.CommitStatusSuccess // if all statuses are skipped, return success
}
- return true
+ return status.State
}
- for _, ctx := range requiredContexts {
- var found bool
- for _, commitStatus := range commitStatuses {
- if commitStatus.Context == ctx {
- if commitStatus.State != structs.CommitStatusSuccess {
- return false
- }
-
- found = true
- break
- }
- }
- if !found {
- return false
- }
- }
- return true
+ return returnedStatus
}
// IsPullCommitStatusPass returns if all required status checks PASS
diff --git a/services/pull/commit_status_test.go b/services/pull/commit_status_test.go
index 592acdd55c..9cb20d6c5d 100644
--- a/services/pull/commit_status_test.go
+++ b/services/pull/commit_status_test.go
@@ -14,52 +14,70 @@ import (
)
func TestMergeRequiredContextsCommitStatus(t *testing.T) {
- testCases := [][]*git_model.CommitStatus{
+ cases := []struct {
+ commitStatuses []*git_model.CommitStatus
+ requiredContexts []string
+ expected structs.CommitStatusState
+ }{
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 3", State: structs.CommitStatusSuccess},
+ commitStatuses: []*git_model.CommitStatus{},
+ requiredContexts: []string{},
+ expected: structs.CommitStatusPending,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusPending},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build xxx", State: structs.CommitStatusSkipped},
+ },
+ requiredContexts: []string{"Build*"},
+ expected: structs.CommitStatusSuccess,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusFailure},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: structs.CommitStatusSkipped},
+ {Context: "Build 2", State: structs.CommitStatusSuccess},
+ {Context: "Build 3", State: structs.CommitStatusSuccess},
+ },
+ requiredContexts: []string{"Build*"},
+ expected: structs.CommitStatusSuccess,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusSuccess},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: structs.CommitStatusSuccess},
+ {Context: "Build 2", State: structs.CommitStatusSuccess},
+ {Context: "Build 2t", State: structs.CommitStatusPending},
+ },
+ requiredContexts: []string{"Build*", "Build 2t*"},
+ expected: structs.CommitStatusPending,
},
{
- {Context: "Build 1", State: structs.CommitStatusSuccess},
- {Context: "Build 2", State: structs.CommitStatusSuccess},
- {Context: "Build 2t", State: structs.CommitStatusSuccess},
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: structs.CommitStatusSuccess},
+ {Context: "Build 2", State: structs.CommitStatusSuccess},
+ {Context: "Build 2t", State: structs.CommitStatusFailure},
+ },
+ requiredContexts: []string{"Build*", "Build 2t*"},
+ expected: structs.CommitStatusFailure,
+ },
+ {
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: structs.CommitStatusSuccess},
+ {Context: "Build 2", State: structs.CommitStatusSuccess},
+ {Context: "Build 2t", State: structs.CommitStatusSuccess},
+ },
+ requiredContexts: []string{"Build*", "Build 2t*", "Build 3*"},
+ expected: structs.CommitStatusPending,
+ },
+ {
+ commitStatuses: []*git_model.CommitStatus{
+ {Context: "Build 1", State: structs.CommitStatusSuccess},
+ {Context: "Build 2", State: structs.CommitStatusSuccess},
+ {Context: "Build 2t", State: structs.CommitStatusSuccess},
+ },
+ requiredContexts: []string{"Build*", "Build *", "Build 2t*", "Build 1*"},
+ expected: structs.CommitStatusSuccess,
},
}
- testCasesRequiredContexts := [][]string{
- {"Build*"},
- {"Build*", "Build 2t*"},
- {"Build*", "Build 2t*"},
- {"Build*", "Build 2t*", "Build 3*"},
- {"Build*", "Build *", "Build 2t*", "Build 1*"},
- }
-
- testCasesExpected := []structs.CommitStatusState{
- structs.CommitStatusSuccess,
- structs.CommitStatusPending,
- structs.CommitStatusFailure,
- structs.CommitStatusPending,
- structs.CommitStatusSuccess,
- }
-
- for i, commitStatuses := range testCases {
- if MergeRequiredContextsCommitStatus(commitStatuses, testCasesRequiredContexts[i]) != testCasesExpected[i] {
- assert.Fail(t, "Test case failed", "Test case %d failed", i+1)
- }
+ for i, c := range cases {
+ assert.Equal(t, c.expected, MergeRequiredContextsCommitStatus(c.commitStatuses, c.requiredContexts), "case %d", i)
}
}
diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl
index eb700ab2bb..7184f5f8eb 100644
--- a/templates/repo/commit_status.tmpl
+++ b/templates/repo/commit_status.tmpl
@@ -14,3 +14,6 @@
{{if eq .State "warning"}}
{{svg "gitea-exclamation" 18 "commit-status icon text yellow"}}
{{end}}
+{{if eq .State "skipped"}}
+ {{svg "octicon-skip" 18 "commit-status icon text grey"}}
+{{end}}
diff --git a/web_src/js/components/DashboardRepoList.vue b/web_src/js/components/DashboardRepoList.vue
index fc6a7bd281..0528ceaf90 100644
--- a/web_src/js/components/DashboardRepoList.vue
+++ b/web_src/js/components/DashboardRepoList.vue
@@ -6,7 +6,7 @@ import {fomanticQuery} from '../modules/fomantic/base.ts';
const {appSubUrl, assetUrlPrefix, pageData} = window.config;
-type CommitStatus = 'pending' | 'success' | 'error' | 'failure' | 'warning';
+type CommitStatus = 'pending' | 'success' | 'error' | 'failure' | 'warning' | 'skipped';
type CommitStatusMap = {
[status in CommitStatus]: {
@@ -22,6 +22,7 @@ const commitStatus: CommitStatusMap = {
error: {name: 'gitea-exclamation', color: 'red'},
failure: {name: 'octicon-x', color: 'red'},
warning: {name: 'gitea-exclamation', color: 'yellow'},
+ skipped: {name: 'octicon-skip', color: 'grey'},
};
export default defineComponent({