From 2130399e4e897f1b79f8684d0ce5d90bb528447b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 28 May 2025 13:00:43 -0700 Subject: [PATCH 1/7] Refactor commit status state --- models/git/commit_status.go | 57 +++-- models/git/commit_status_summary.go | 12 +- models/git/commit_status_test.go | 70 +++--- .../commit_status.go | 49 +++-- modules/commitstatus/commit_status_test.go | 201 ++++++++++++++++++ modules/structs/commit_status_test.go | 30 --- modules/structs/status.go | 38 ++-- services/actions/commit_status.go | 14 +- services/convert/status.go | 43 ++-- services/pull/commit_status.go | 75 +++---- services/pull/commit_status_test.go | 50 ++--- .../repository/commitstatus/commitstatus.go | 6 +- tests/integration/actions_trigger_test.go | 7 +- tests/integration/git_general_test.go | 9 +- tests/integration/pull_merge_test.go | 7 +- tests/integration/pull_status_test.go | 27 +-- tests/integration/repo_commits_test.go | 11 +- tests/integration/repo_webhook_test.go | 3 +- 18 files changed, 437 insertions(+), 272 deletions(-) rename modules/{structs => commitstatus}/commit_status.go (60%) create mode 100644 modules/commitstatus/commit_status_test.go delete mode 100644 modules/structs/commit_status_test.go diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 2e765391b899c..f85e1b15e5600 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -17,10 +17,10 @@ import ( "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/translation" @@ -30,17 +30,17 @@ import ( // CommitStatus holds a single Status of a single Commit type CommitStatus struct { - ID int64 `xorm:"pk autoincr"` - Index int64 `xorm:"INDEX UNIQUE(repo_sha_index)"` - RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_index)"` - Repo *repo_model.Repository `xorm:"-"` - State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"` - SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"` - TargetURL string `xorm:"TEXT"` - Description string `xorm:"TEXT"` - ContextHash string `xorm:"VARCHAR(64) index"` - Context string `xorm:"TEXT"` - Creator *user_model.User `xorm:"-"` + ID int64 `xorm:"pk autoincr"` + Index int64 `xorm:"INDEX UNIQUE(repo_sha_index)"` + RepoID int64 `xorm:"INDEX UNIQUE(repo_sha_index)"` + Repo *repo_model.Repository `xorm:"-"` + State commitstatus.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"` + SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_sha_index)"` + TargetURL string `xorm:"TEXT"` + Description string `xorm:"TEXT"` + ContextHash string `xorm:"VARCHAR(64) index"` + Context string `xorm:"TEXT"` + Creator *user_model.User `xorm:"-"` CreatorID int64 CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` @@ -230,28 +230,25 @@ 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 + if len(statuses) == 0 { + return nil + } + + states := make(commitstatus.CommitStatusStates, 0, len(statuses)) + targetURL := "" for _, status := range statuses { - if state == status.State || status.State.HasHigherPriorityThan(state) { - state = status.State - lastStatus = status + states = append(states, status.State) + if status.TargetURL != "" { + targetURL = status.TargetURL } } - 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{} - } + + return &CommitStatus{ + RepoID: statuses[0].RepoID, + SHA: statuses[0].SHA, + State: states.Combine(), + TargetURL: targetURL, } - return lastStatus } // CommitStatusOptions holds the options for query commit statuses diff --git a/models/git/commit_status_summary.go b/models/git/commit_status_summary.go index 774e49bb9859b..dd416fa015985 100644 --- a/models/git/commit_status_summary.go +++ b/models/git/commit_status_summary.go @@ -7,19 +7,19 @@ import ( "context" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/setting" - api "code.gitea.io/gitea/modules/structs" "xorm.io/builder" ) // CommitStatusSummary holds the latest commit Status of a single Commit type CommitStatusSummary struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX UNIQUE(repo_id_sha)"` - SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_id_sha)"` - State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"` - TargetURL string `xorm:"TEXT"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX UNIQUE(repo_id_sha)"` + SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_id_sha)"` + State commitstatus.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"` + TargetURL string `xorm:"TEXT"` } func init() { diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index a01d030e71dde..f7c8e30beb573 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -14,9 +14,9 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" ) @@ -38,23 +38,23 @@ func TestGetCommitStatuses(t *testing.T) { assert.Len(t, statuses, 5) assert.Equal(t, "ci/awesomeness", statuses[0].Context) - assert.Equal(t, structs.CommitStatusPending, statuses[0].State) + assert.Equal(t, commitstatus.CommitStatusPending, statuses[0].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[0].APIURL(db.DefaultContext)) assert.Equal(t, "cov/awesomeness", statuses[1].Context) - assert.Equal(t, structs.CommitStatusWarning, statuses[1].State) + assert.Equal(t, commitstatus.CommitStatusWarning, statuses[1].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[1].APIURL(db.DefaultContext)) assert.Equal(t, "cov/awesomeness", statuses[2].Context) - assert.Equal(t, structs.CommitStatusSuccess, statuses[2].State) + assert.Equal(t, commitstatus.CommitStatusSuccess, statuses[2].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[2].APIURL(db.DefaultContext)) assert.Equal(t, "ci/awesomeness", statuses[3].Context) - assert.Equal(t, structs.CommitStatusFailure, statuses[3].State) + assert.Equal(t, commitstatus.CommitStatusFailure, statuses[3].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[3].APIURL(db.DefaultContext)) assert.Equal(t, "deploy/awesomeness", statuses[4].Context) - assert.Equal(t, structs.CommitStatusError, statuses[4].State) + assert.Equal(t, commitstatus.CommitStatusError, statuses[4].State) assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[4].APIURL(db.DefaultContext)) statuses, maxResults, err = db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{ @@ -75,104 +75,104 @@ func Test_CalcCommitStatus(t *testing.T) { { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, }, { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, { - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, }, { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, { - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, }, { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusError, + State: commitstatus.CommitStatusError, }, { - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusError, + State: commitstatus.CommitStatusError, }, }, { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusWarning, + State: commitstatus.CommitStatusWarning, }, { - State: structs.CommitStatusPending, + State: commitstatus.CommitStatusPending, }, { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusWarning, + State: commitstatus.CommitStatusWarning, }, }, { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, { - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, }, }, { statuses: []*git_model.CommitStatus{ { - State: structs.CommitStatusFailure, + State: commitstatus.CommitStatusFailure, }, { - State: structs.CommitStatusError, + State: commitstatus.CommitStatusError, }, { - State: structs.CommitStatusWarning, + State: commitstatus.CommitStatusWarning, }, }, expected: &git_model.CommitStatus{ - State: structs.CommitStatusError, + State: commitstatus.CommitStatusError, }, }, } @@ -208,7 +208,7 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) { Creator: user2, SHA: commit.ID, CommitStatus: &git_model.CommitStatus{ - State: structs.CommitStatusFailure, + State: commitstatus.CommitStatusFailure, TargetURL: "https://example.com/tests/", Context: "compliance/lint-backend", }, @@ -220,7 +220,7 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) { Creator: user2, SHA: commit.ID, CommitStatus: &git_model.CommitStatus{ - State: structs.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "https://example.com/tests/", Context: "compliance/lint-backend", }, @@ -270,9 +270,9 @@ func TestGetCountLatestCommitStatus(t *testing.T) { }) assert.NoError(t, err) assert.Len(t, commitStatuses, 2) - assert.Equal(t, structs.CommitStatusFailure, commitStatuses[0].State) + assert.Equal(t, commitstatus.CommitStatusFailure, commitStatuses[0].State) assert.Equal(t, "ci/awesomeness", commitStatuses[0].Context) - assert.Equal(t, structs.CommitStatusError, commitStatuses[1].State) + assert.Equal(t, commitstatus.CommitStatusError, commitStatuses[1].State) assert.Equal(t, "deploy/awesomeness", commitStatuses[1].Context) count, err := git_model.CountLatestCommitStatus(db.DefaultContext, repo1.ID, sha1) diff --git a/modules/structs/commit_status.go b/modules/commitstatus/commit_status.go similarity index 60% rename from modules/structs/commit_status.go rename to modules/commitstatus/commit_status.go index 398001974d4c4..13f7ca773c098 100644 --- a/modules/structs/commit_status.go +++ b/modules/commitstatus/commit_status.go @@ -1,11 +1,10 @@ // Copyright 2020 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package structs +package commitstatus // CommitStatusState holds the state of a CommitStatus -// It can be "pending", "success", "error" and "failure" -type CommitStatusState string +type CommitStatusState string //nolint const ( // CommitStatusPending is for when the CommitStatus is Pending @@ -22,25 +21,10 @@ const ( CommitStatusSkipped CommitStatusState = "skipped" ) -var commitStatusPriorities = map[CommitStatusState]int{ - CommitStatusError: 0, - CommitStatusFailure: 1, - CommitStatusWarning: 2, - CommitStatusPending: 3, - CommitStatusSuccess: 4, - CommitStatusSkipped: 5, -} - func (css CommitStatusState) String() string { return string(css) } -// 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 func (css CommitStatusState) IsPending() bool { return css == CommitStatusPending @@ -65,3 +49,32 @@ func (css CommitStatusState) IsFailure() bool { func (css CommitStatusState) IsWarning() bool { return css == CommitStatusWarning } + +// IsSkipped represents if commit status state is skipped +func (css CommitStatusState) IsSkipped() bool { + return css == CommitStatusSkipped +} + +type CommitStatusStates []CommitStatusState //nolint + +// According to https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#get-the-combined-status-for-a-specific-reference +// > Additionally, a combined state is returned. The state is one of: +// > 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 +func (css CommitStatusStates) Combine() CommitStatusState { + successCnt := 0 + for _, state := range css { + switch { + case state.IsError() || state.IsFailure(): + return CommitStatusFailure + case state.IsPending(): + case state.IsSuccess() || state.IsWarning() || state.IsSkipped(): + successCnt++ + } + } + if successCnt > 0 && successCnt == len(css) { + return CommitStatusSuccess + } + return CommitStatusPending +} diff --git a/modules/commitstatus/commit_status_test.go b/modules/commitstatus/commit_status_test.go new file mode 100644 index 0000000000000..10d8f20aa4830 --- /dev/null +++ b/modules/commitstatus/commit_status_test.go @@ -0,0 +1,201 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package commitstatus + +import "testing" + +func TestCombine(t *testing.T) { + tests := []struct { + name string + states CommitStatusStates + expected CommitStatusState + }{ + // 0 states + { + name: "empty", + states: CommitStatusStates{}, + expected: CommitStatusPending, + }, + // 1 state + { + name: "pending", + states: CommitStatusStates{CommitStatusPending}, + expected: CommitStatusPending, + }, + { + name: "success", + states: CommitStatusStates{CommitStatusSuccess}, + expected: CommitStatusSuccess, + }, + { + name: "error", + states: CommitStatusStates{CommitStatusError}, + expected: CommitStatusFailure, + }, + { + name: "failure", + states: CommitStatusStates{CommitStatusFailure}, + expected: CommitStatusFailure, + }, + { + name: "warning", + states: CommitStatusStates{CommitStatusWarning}, + expected: CommitStatusSuccess, + }, + // 2 states + { + name: "pending and success", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess}, + expected: CommitStatusPending, + }, + { + name: "pending and error", + states: CommitStatusStates{CommitStatusPending, CommitStatusError}, + expected: CommitStatusFailure, + }, + { + name: "pending and failure", + states: CommitStatusStates{CommitStatusPending, CommitStatusFailure}, + expected: CommitStatusFailure, + }, + { + name: "pending and warning", + states: CommitStatusStates{CommitStatusPending, CommitStatusWarning}, + expected: CommitStatusPending, + }, + { + name: "success and error", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusError}, + expected: CommitStatusFailure, + }, + { + name: "success and failure", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusFailure}, + expected: CommitStatusFailure, + }, + { + name: "success and warning", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusWarning}, + expected: CommitStatusSuccess, + }, + { + name: "error and failure", + states: CommitStatusStates{CommitStatusError, CommitStatusFailure}, + expected: CommitStatusFailure, + }, + { + name: "error and warning", + states: CommitStatusStates{CommitStatusError, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "failure and warning", + states: CommitStatusStates{CommitStatusFailure, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + // 3 states + { + name: "pending, success and warning", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusWarning}, + expected: CommitStatusPending, + }, + { + name: "pending, success and error", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusError}, + expected: CommitStatusFailure, + }, + { + name: "pending, success and failure", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusFailure}, + expected: CommitStatusFailure, + }, + { + name: "pending, error and failure", + states: CommitStatusStates{CommitStatusPending, CommitStatusError, CommitStatusFailure}, + expected: CommitStatusFailure, + }, + { + name: "success, error and warning", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusError, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "success, failure and warning", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusFailure, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "error, failure and warning", + states: CommitStatusStates{CommitStatusError, CommitStatusFailure, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "success, warning and skipped", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusWarning, CommitStatusSkipped}, + expected: CommitStatusSuccess, + }, + // All success + { + name: "all success", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusSuccess, CommitStatusSuccess}, + expected: CommitStatusSuccess, + }, + // All pending + { + name: "all pending", + states: CommitStatusStates{CommitStatusPending, CommitStatusPending, CommitStatusPending}, + expected: CommitStatusPending, + }, + { + name: "all skipped", + states: CommitStatusStates{CommitStatusSkipped, CommitStatusSkipped, CommitStatusSkipped}, + expected: CommitStatusSuccess, + }, + // 4 states + { + name: "pending, success, error and warning", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusError, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "pending, success, failure and warning", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusFailure, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "pending, error, failure and warning", + states: CommitStatusStates{CommitStatusPending, CommitStatusError, CommitStatusFailure, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "success, error, failure and warning", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusError, CommitStatusFailure, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "mixed states", + states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusError, CommitStatusWarning}, + expected: CommitStatusFailure, + }, + { + name: "mixed states with all success", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusSuccess, CommitStatusPending, CommitStatusWarning}, + expected: CommitStatusPending, + }, + { + name: "all success with warning", + states: CommitStatusStates{CommitStatusSuccess, CommitStatusSuccess, CommitStatusSuccess, CommitStatusWarning}, + expected: CommitStatusSuccess, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.states.Combine() + if result != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) + } +} diff --git a/modules/structs/commit_status_test.go b/modules/structs/commit_status_test.go deleted file mode 100644 index c11daf248d1c3..0000000000000 --- a/modules/structs/commit_status_test.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package structs - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestNoBetterThan(t *testing.T) { - tests := []struct { - s1, s2 CommitStatusState - higher bool - }{ - {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 { - 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/modules/structs/status.go b/modules/structs/status.go index c1d8b902ec3a7..a9779541ff20a 100644 --- a/modules/structs/status.go +++ b/modules/structs/status.go @@ -5,17 +5,19 @@ package structs import ( "time" + + "code.gitea.io/gitea/modules/commitstatus" ) // CommitStatus holds a single status of a single Commit type CommitStatus struct { - ID int64 `json:"id"` - State CommitStatusState `json:"status"` - TargetURL string `json:"target_url"` - Description string `json:"description"` - URL string `json:"url"` - Context string `json:"context"` - Creator *User `json:"creator"` + ID int64 `json:"id"` + State commitstatus.CommitStatusState `json:"status"` + TargetURL string `json:"target_url"` + Description string `json:"description"` + URL string `json:"url"` + Context string `json:"context"` + Creator *User `json:"creator"` // swagger:strfmt date-time Created time.Time `json:"created_at"` // swagger:strfmt date-time @@ -24,19 +26,19 @@ type CommitStatus struct { // CombinedStatus holds the combined state of several statuses for a single commit type CombinedStatus struct { - State CommitStatusState `json:"state"` - SHA string `json:"sha"` - TotalCount int `json:"total_count"` - Statuses []*CommitStatus `json:"statuses"` - Repository *Repository `json:"repository"` - CommitURL string `json:"commit_url"` - URL string `json:"url"` + State commitstatus.CommitStatusState `json:"state"` + SHA string `json:"sha"` + TotalCount int `json:"total_count"` + Statuses []*CommitStatus `json:"statuses"` + Repository *Repository `json:"repository"` + CommitURL string `json:"commit_url"` + URL string `json:"url"` } // CreateStatusOption holds the information needed to create a new CommitStatus for a Commit type CreateStatusOption struct { - State CommitStatusState `json:"state"` - TargetURL string `json:"target_url"` - Description string `json:"description"` - Context string `json:"context"` + State commitstatus.CommitStatusState `json:"state"` + TargetURL string `json:"target_url"` + Description string `json:"description"` + Context string `json:"context"` } diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 5d17df8047c12..ef241e5091a2f 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -14,9 +14,9 @@ import ( git_model "code.gitea.io/gitea/models/git" user_model "code.gitea.io/gitea/models/user" actions_module "code.gitea.io/gitea/modules/actions" + "code.gitea.io/gitea/modules/commitstatus" git "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus" @@ -147,18 +147,18 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er return commitstatus_service.CreateCommitStatus(ctx, repo, creator, commitID.String(), &status) } -func toCommitStatus(status actions_model.Status) api.CommitStatusState { +func toCommitStatus(status actions_model.Status) commitstatus.CommitStatusState { switch status { case actions_model.StatusSuccess: - return api.CommitStatusSuccess + return commitstatus.CommitStatusSuccess case actions_model.StatusFailure, actions_model.StatusCancelled: - return api.CommitStatusFailure + return commitstatus.CommitStatusFailure case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning: - return api.CommitStatusPending + return commitstatus.CommitStatusPending case actions_model.StatusSkipped: - return api.CommitStatusSkipped + return commitstatus.CommitStatusSkipped default: - return api.CommitStatusError + return commitstatus.CommitStatusError } } diff --git a/services/convert/status.go b/services/convert/status.go index 4fffbfbe5ee6f..b4864a0307b7e 100644 --- a/services/convert/status.go +++ b/services/convert/status.go @@ -5,6 +5,7 @@ package convert import ( "context" + "net/url" git_model "code.gitea.io/gitea/models/git" user_model "code.gitea.io/gitea/models/user" @@ -32,39 +33,29 @@ func ToCommitStatus(ctx context.Context, status *git_model.CommitStatus) *api.Co return apiStatus } +func ToCommitStatuses(ctx context.Context, statuses []*git_model.CommitStatus) []*api.CommitStatus { + apiStatuses := make([]*api.CommitStatus, len(statuses)) + for i, status := range statuses { + apiStatuses[i] = ToCommitStatus(ctx, status) + } + return apiStatuses +} + // ToCombinedStatus converts List of CommitStatus to a CombinedStatus func ToCombinedStatus(ctx context.Context, statuses []*git_model.CommitStatus, repo *api.Repository) *api.CombinedStatus { if len(statuses) == 0 { return nil } - retStatus := &api.CombinedStatus{ - SHA: statuses[0].SHA, + combinedStatus := git_model.CalcCommitStatus(statuses) + + return &api.CombinedStatus{ + State: combinedStatus.State, + Statuses: ToCommitStatuses(ctx, statuses), + SHA: combinedStatus.SHA, TotalCount: len(statuses), Repository: repo, - 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 status.State.HasHigherPriorityThan(retStatus.State) { - retStatus.State = status.State - } - } - // According to https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#get-the-combined-status-for-a-specific-reference - // > Additionally, a combined state is returned. The state is one of: - // > 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 - 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 + CommitURL: repo.URL + "/commits/" + url.PathEscape(combinedStatus.SHA), + URL: repo.URL + "/commits/" + url.PathEscape(combinedStatus.SHA) + "/status", } - return retStatus } diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index 58d26c5a00079..ea95a2c63a78e 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -10,67 +10,52 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/structs" "github.com/gobwas/glob" "github.com/pkg/errors" ) // MergeRequiredContextsCommitStatus returns a commit status state for given required contexts -func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, requiredContexts []string) structs.CommitStatusState { - // matchedCount is the number of `CommitStatus.Context` that match any context of `requiredContexts` - matchedCount := 0 - returnedStatus := structs.CommitStatusSuccess - - if len(requiredContexts) > 0 { - requiredContextsGlob := make(map[string]glob.Glob, len(requiredContexts)) - for _, ctx := range requiredContexts { - if gp, err := glob.Compile(ctx); err != nil { - log.Error("glob.Compile %s failed. Error: %v", ctx, err) - } else { - requiredContextsGlob[ctx] = gp - } - } +func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, requiredContexts []string) commitstatus.CommitStatusState { + if len(requiredContexts) == 0 { + return git_model.CalcCommitStatus(commitStatuses).State + } - for _, gp := range requiredContextsGlob { - var targetStatus structs.CommitStatusState - for _, commitStatus := range commitStatuses { - if gp.Match(commitStatus.Context) { - targetStatus = commitStatus.State - matchedCount++ - break - } - } + requiredContextsGlob := make(map[string]glob.Glob, len(requiredContexts)) + for _, ctx := range requiredContexts { + if gp, err := glob.Compile(ctx); err != nil { + log.Error("glob.Compile %s failed. Error: %v", ctx, err) + } else { + requiredContextsGlob[ctx] = gp + } + } - // If required rule not match any action, then it is pending - if targetStatus == "" { - if structs.CommitStatusPending.HasHigherPriorityThan(returnedStatus) { - returnedStatus = structs.CommitStatusPending - } + requiredCommitStatuses := make([]*git_model.CommitStatus, 0, len(commitStatuses)) + for _, gp := range requiredContextsGlob { + for _, commitStatus := range commitStatuses { + if gp.Match(commitStatus.Context) { + requiredCommitStatuses = append(requiredCommitStatuses, commitStatus) break } - - if targetStatus.HasHigherPriorityThan(returnedStatus) { - returnedStatus = targetStatus - } } } + if len(requiredCommitStatuses) == 0 { + return commitstatus.CommitStatusPending + } - if matchedCount == 0 && returnedStatus == structs.CommitStatusSuccess { - if len(commitStatuses) == 0 { - // "no statuses" should mean "pending" - return structs.CommitStatusPending - } - status := git_model.CalcCommitStatus(commitStatuses) - if status.State == structs.CommitStatusSkipped { - return structs.CommitStatusSuccess // if all statuses are skipped, return success - } - return status.State + returnedStatus := git_model.CalcCommitStatus(requiredCommitStatuses).State + if len(requiredCommitStatuses) == len(requiredContexts) { + return returnedStatus } - return returnedStatus + if returnedStatus == commitstatus.CommitStatusFailure { + return commitstatus.CommitStatusFailure + } + // even if part of success, return pending + return commitstatus.CommitStatusPending } // IsPullCommitStatusPass returns if all required status checks PASS @@ -91,7 +76,7 @@ func IsPullCommitStatusPass(ctx context.Context, pr *issues_model.PullRequest) ( } // GetPullRequestCommitStatusState returns pull request merged commit status state -func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullRequest) (structs.CommitStatusState, error) { +func GetPullRequestCommitStatusState(ctx context.Context, pr *issues_model.PullRequest) (commitstatus.CommitStatusState, error) { // Ensure HeadRepo is loaded if err := pr.LoadHeadRepo(ctx); err != nil { return "", errors.Wrap(err, "LoadHeadRepo") diff --git a/services/pull/commit_status_test.go b/services/pull/commit_status_test.go index 9cb20d6c5d892..b985a9de8e4c3 100644 --- a/services/pull/commit_status_test.go +++ b/services/pull/commit_status_test.go @@ -8,7 +8,7 @@ import ( "testing" git_model "code.gitea.io/gitea/models/git" - "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/commitstatus" "github.com/stretchr/testify/assert" ) @@ -17,64 +17,64 @@ func TestMergeRequiredContextsCommitStatus(t *testing.T) { cases := []struct { commitStatuses []*git_model.CommitStatus requiredContexts []string - expected structs.CommitStatusState + expected commitstatus.CommitStatusState }{ { commitStatuses: []*git_model.CommitStatus{}, requiredContexts: []string{}, - expected: structs.CommitStatusPending, + expected: commitstatus.CommitStatusPending, }, { commitStatuses: []*git_model.CommitStatus{ - {Context: "Build xxx", State: structs.CommitStatusSkipped}, + {Context: "Build xxx", State: commitstatus.CommitStatusSkipped}, }, requiredContexts: []string{"Build*"}, - expected: structs.CommitStatusSuccess, + expected: commitstatus.CommitStatusSuccess, }, { commitStatuses: []*git_model.CommitStatus{ - {Context: "Build 1", State: structs.CommitStatusSkipped}, - {Context: "Build 2", State: structs.CommitStatusSuccess}, - {Context: "Build 3", State: structs.CommitStatusSuccess}, + {Context: "Build 1", State: commitstatus.CommitStatusSkipped}, + {Context: "Build 2", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 3", State: commitstatus.CommitStatusSuccess}, }, requiredContexts: []string{"Build*"}, - expected: structs.CommitStatusSuccess, + expected: commitstatus.CommitStatusSuccess, }, { commitStatuses: []*git_model.CommitStatus{ - {Context: "Build 1", State: structs.CommitStatusSuccess}, - {Context: "Build 2", State: structs.CommitStatusSuccess}, - {Context: "Build 2t", State: structs.CommitStatusPending}, + {Context: "Build 1", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2t", State: commitstatus.CommitStatusPending}, }, requiredContexts: []string{"Build*", "Build 2t*"}, - expected: structs.CommitStatusPending, + expected: commitstatus.CommitStatusPending, }, { commitStatuses: []*git_model.CommitStatus{ - {Context: "Build 1", State: structs.CommitStatusSuccess}, - {Context: "Build 2", State: structs.CommitStatusSuccess}, - {Context: "Build 2t", State: structs.CommitStatusFailure}, + {Context: "Build 1", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2t", State: commitstatus.CommitStatusFailure}, }, requiredContexts: []string{"Build*", "Build 2t*"}, - expected: structs.CommitStatusFailure, + expected: commitstatus.CommitStatusFailure, }, { commitStatuses: []*git_model.CommitStatus{ - {Context: "Build 1", State: structs.CommitStatusSuccess}, - {Context: "Build 2", State: structs.CommitStatusSuccess}, - {Context: "Build 2t", State: structs.CommitStatusSuccess}, + {Context: "Build 1", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2t", State: commitstatus.CommitStatusSuccess}, }, requiredContexts: []string{"Build*", "Build 2t*", "Build 3*"}, - expected: structs.CommitStatusPending, + expected: commitstatus.CommitStatusPending, }, { commitStatuses: []*git_model.CommitStatus{ - {Context: "Build 1", State: structs.CommitStatusSuccess}, - {Context: "Build 2", State: structs.CommitStatusSuccess}, - {Context: "Build 2t", State: structs.CommitStatusSuccess}, + {Context: "Build 1", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2", State: commitstatus.CommitStatusSuccess}, + {Context: "Build 2t", State: commitstatus.CommitStatusSuccess}, }, requiredContexts: []string{"Build*", "Build *", "Build 2t*", "Build 1*"}, - expected: structs.CommitStatusSuccess, + expected: commitstatus.CommitStatusSuccess, }, } for i, c := range cases { diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index f369a303e6ce4..44cf61df43344 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -14,12 +14,12 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" repo_module "code.gitea.io/gitea/modules/repository" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/services/notify" ) @@ -47,7 +47,7 @@ func getCommitStatusCache(repoID int64, branchName string) *commitStatusCacheVal return nil } -func updateCommitStatusCache(repoID int64, branchName string, state api.CommitStatusState, targetURL string) error { +func updateCommitStatusCache(repoID int64, branchName string, state commitstatus.CommitStatusState, targetURL string) error { c := cache.GetCache() bs, err := json.Marshal(commitStatusCacheValue{ State: state.String(), @@ -127,7 +127,7 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep for i, repo := range repos { if cv := getCommitStatusCache(repo.ID, repo.DefaultBranch); cv != nil { results[i] = &git_model.CommitStatus{ - State: api.CommitStatusState(cv.State), + State: commitstatus.CommitStatusState(cv.State), TargetURL: cv.TargetURL, } } else { diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index a598cf64a5b45..6461fe85f4d16 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" actions_module "code.gitea.io/gitea/modules/actions" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/json" @@ -638,7 +639,7 @@ jobs: if len(latestCommitStatuses) == 0 { return false } - if latestCommitStatuses[0].State == api.CommitStatusPending { + if latestCommitStatuses[0].State == commitstatus.CommitStatusPending { insertFakeStatus(t, repo, sha, latestCommitStatuses[0].TargetURL, latestCommitStatuses[0].Context) return true } @@ -679,14 +680,14 @@ func checkCommitStatusAndInsertFakeStatus(t *testing.T, repo *repo_model.Reposit latestCommitStatuses, err := git_model.GetLatestCommitStatus(db.DefaultContext, repo.ID, sha, db.ListOptionsAll) assert.NoError(t, err) assert.Len(t, latestCommitStatuses, 1) - assert.Equal(t, api.CommitStatusPending, latestCommitStatuses[0].State) + assert.Equal(t, commitstatus.CommitStatusPending, latestCommitStatuses[0].State) insertFakeStatus(t, repo, sha, latestCommitStatuses[0].TargetURL, latestCommitStatuses[0].Context) } func insertFakeStatus(t *testing.T, repo *repo_model.Repository, sha, targetURL, context string) { err := commitstatus_service.CreateCommitStatus(db.DefaultContext, repo, user_model.NewActionsUser(), sha, &git_model.CommitStatus{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: targetURL, Context: context, }) diff --git a/tests/integration/git_general_test.go b/tests/integration/git_general_test.go index ed60bdb58a432..3b0f9589d2c63 100644 --- a/tests/integration/git_general_test.go +++ b/tests/integration/git_general_test.go @@ -26,6 +26,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/setting" @@ -713,7 +714,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { commitID := path.Base(commitURL) - addCommitStatus := func(status api.CommitStatusState) func(*testing.T) { + addCommitStatus := func(status commitstatus.CommitStatusState) func(*testing.T) { return doAPICreateCommitStatus(ctx, commitID, api.CreateStatusOption{ State: status, TargetURL: "http://test.ci/", @@ -723,7 +724,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { } // Call API to add Pending status for commit - t.Run("CreateStatus", addCommitStatus(api.CommitStatusPending)) + t.Run("CreateStatus", addCommitStatus(commitstatus.CommitStatusPending)) // Cancel not existing auto merge ctx.ExpectedCode = http.StatusNotFound @@ -752,7 +753,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { assert.False(t, pr.HasMerged) // Call API to add Failure status for commit - t.Run("CreateStatus", addCommitStatus(api.CommitStatusFailure)) + t.Run("CreateStatus", addCommitStatus(commitstatus.CommitStatusFailure)) // Check pr status pr, err = doAPIGetPullRequest(ctx, baseCtx.Username, baseCtx.Reponame, pr.Index)(t) @@ -760,7 +761,7 @@ func doAutoPRMerge(baseCtx *APITestContext, dstPath string) func(t *testing.T) { assert.False(t, pr.HasMerged) // Call API to add Success status for commit - t.Run("CreateStatus", addCommitStatus(api.CommitStatusSuccess)) + t.Run("CreateStatus", addCommitStatus(commitstatus.CommitStatusSuccess)) // wait to let gitea merge stuff time.Sleep(time.Second) diff --git a/tests/integration/pull_merge_test.go b/tests/integration/pull_merge_test.go index cf50d5e639e2d..5fad26f70e745 100644 --- a/tests/integration/pull_merge_test.go +++ b/tests/integration/pull_merge_test.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/queue" @@ -768,7 +769,7 @@ func TestPullAutoMergeAfterCommitStatusSucceed(t *testing.T) { }() err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "https://gitea.com", Context: "gitea/actions", }) @@ -848,7 +849,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApproval(t *testing.T) { }() err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "https://gitea.com", Context: "gitea/actions", }) @@ -977,7 +978,7 @@ func TestPullAutoMergeAfterCommitStatusSucceedAndApprovalForAgitFlow(t *testing. }() err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "https://gitea.com", Context: "gitea/actions", }) diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 4d43847f1bee7..00ffcfb632c0c 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" @@ -55,20 +56,20 @@ func TestPullCreate_CommitStatus(t *testing.T) { commitID := path.Base(commitURL) - statusList := []api.CommitStatusState{ - api.CommitStatusPending, - api.CommitStatusError, - api.CommitStatusFailure, - api.CommitStatusSuccess, - api.CommitStatusWarning, + statusList := []commitstatus.CommitStatusState{ + commitstatus.CommitStatusPending, + commitstatus.CommitStatusError, + commitstatus.CommitStatusFailure, + commitstatus.CommitStatusSuccess, + commitstatus.CommitStatusWarning, } - statesIcons := map[api.CommitStatusState]string{ - api.CommitStatusPending: "octicon-dot-fill", - api.CommitStatusSuccess: "octicon-check", - api.CommitStatusError: "gitea-exclamation", - api.CommitStatusFailure: "octicon-x", - api.CommitStatusWarning: "gitea-exclamation", + statesIcons := map[commitstatus.CommitStatusState]string{ + commitstatus.CommitStatusPending: "octicon-dot-fill", + commitstatus.CommitStatusSuccess: "octicon-check", + commitstatus.CommitStatusError: "gitea-exclamation", + commitstatus.CommitStatusFailure: "octicon-x", + commitstatus.CommitStatusWarning: "gitea-exclamation", } testCtx := NewAPITestContext(t, "user1", "repo1", auth_model.AccessTokenScopeWriteRepository) @@ -99,7 +100,7 @@ func TestPullCreate_CommitStatus(t *testing.T) { repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) css := unittest.AssertExistsAndLoadBean(t, &git_model.CommitStatusSummary{RepoID: repo1.ID, SHA: commitID}) - assert.Equal(t, api.CommitStatusWarning, css.State) + assert.Equal(t, commitstatus.CommitStatusWarning, css.State) }) } diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 504d2adaccd8f..bef957597ae62 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -12,6 +12,7 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -76,7 +77,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { // Call API to add status for commit ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository) t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{ - State: api.CommitStatusState(state), + State: commitstatus.CommitStatusState(state), TargetURL: "http://test.ci/", Description: "", Context: "testci", @@ -120,7 +121,7 @@ func testRepoCommitsWithStatus(t *testing.T, resp, respOne *httptest.ResponseRec assert.NotNil(t, status) if assert.Len(t, statuses, 1) { - assert.Equal(t, api.CommitStatusState(state), statuses[0].State) + assert.Equal(t, commitstatus.CommitStatusState(state), statuses[0].State) assert.Equal(t, setting.AppURL+"api/v1/repos/user2/repo1/statuses/65f1bf27bc3bf70f64657658635e66094edbcb4d", statuses[0].URL) assert.Equal(t, "http://test.ci/", statuses[0].TargetURL) assert.Empty(t, statuses[0].Description) @@ -174,7 +175,7 @@ func TestRepoCommitsStatusParallel(t *testing.T) { parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) { ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository) runBody := doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{ - State: api.CommitStatusPending, + State: commitstatus.CommitStatusPending, TargetURL: "http://test.ci/", Description: "", Context: "testci", @@ -205,14 +206,14 @@ func TestRepoCommitsStatusMultiple(t *testing.T) { // Call API to add status for commit ctx := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository) t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "http://test.ci/", Description: "", Context: "testci", })) t.Run("CreateStatus", doAPICreateCommitStatus(ctx, path.Base(commitURL), api.CreateStatusOption{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "http://test.ci/", Description: "", Context: "other_context", diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go index 438dd3211d3c0..77c8e62e9b290 100644 --- a/tests/integration/repo_webhook_test.go +++ b/tests/integration/repo_webhook_test.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/models/webhook" + "code.gitea.io/gitea/modules/commitstatus" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/json" api "code.gitea.io/gitea/modules/structs" @@ -771,7 +772,7 @@ func Test_WebhookStatus(t *testing.T) { // update a status for a commit via API doAPICreateCommitStatus(testCtx, commitID, api.CreateStatusOption{ - State: api.CommitStatusSuccess, + State: commitstatus.CommitStatusSuccess, TargetURL: "http://test.ci/", Description: "", Context: "testci", From 3bd51a51dd9617382a5f73361c86bf32b0a76f4e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 28 May 2025 15:53:30 -0700 Subject: [PATCH 2/7] Fix swagger doc --- modules/commitstatus/commit_status.go | 1 + templates/swagger/v1_json.tmpl | 41 +++++++++++++++++++++------ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/modules/commitstatus/commit_status.go b/modules/commitstatus/commit_status.go index 13f7ca773c098..12004474ed093 100644 --- a/modules/commitstatus/commit_status.go +++ b/modules/commitstatus/commit_status.go @@ -4,6 +4,7 @@ package commitstatus // CommitStatusState holds the state of a CommitStatus +// swagger:enum CommitStatusState type CommitStatusState string //nolint const ( diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 71600e950c7a8..7a2cd6501afbb 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -21030,7 +21030,17 @@ "x-go-name": "SHA" }, "state": { - "$ref": "#/definitions/CommitStatusState" + "type": "string", + "enum": [ + "pending", + "success", + "error", + "failure", + "warning", + "skipped" + ], + "x-go-enum-desc": "pending CommitStatusPending CommitStatusPending is for when the CommitStatus is Pending\nsuccess CommitStatusSuccess CommitStatusSuccess is for when the CommitStatus is Success\nerror CommitStatusError CommitStatusError is for when the CommitStatus is Error\nfailure CommitStatusFailure CommitStatusFailure is for when the CommitStatus is Failure\nwarning CommitStatusWarning CommitStatusWarning is for when the CommitStatus is Warning\nskipped CommitStatusSkipped CommitStatusSkipped is for when CommitStatus is Skipped", + "x-go-name": "State" }, "statuses": { "type": "array", @@ -21258,7 +21268,17 @@ "x-go-name": "ID" }, "status": { - "$ref": "#/definitions/CommitStatusState" + "type": "string", + "enum": [ + "pending", + "success", + "error", + "failure", + "warning", + "skipped" + ], + "x-go-enum-desc": "pending CommitStatusPending CommitStatusPending is for when the CommitStatus is Pending\nsuccess CommitStatusSuccess CommitStatusSuccess is for when the CommitStatus is Success\nerror CommitStatusError CommitStatusError is for when the CommitStatus is Error\nfailure CommitStatusFailure CommitStatusFailure is for when the CommitStatus is Failure\nwarning CommitStatusWarning CommitStatusWarning is for when the CommitStatus is Warning\nskipped CommitStatusSkipped CommitStatusSkipped is for when CommitStatus is Skipped", + "x-go-name": "State" }, "target_url": { "type": "string", @@ -21276,11 +21296,6 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, - "CommitStatusState": { - "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" - }, "CommitUser": { "type": "object", "title": "CommitUser contains information of a user in the context of a commit.", @@ -22329,7 +22344,17 @@ "x-go-name": "Description" }, "state": { - "$ref": "#/definitions/CommitStatusState" + "type": "string", + "enum": [ + "pending", + "success", + "error", + "failure", + "warning", + "skipped" + ], + "x-go-enum-desc": "pending CommitStatusPending CommitStatusPending is for when the CommitStatus is Pending\nsuccess CommitStatusSuccess CommitStatusSuccess is for when the CommitStatus is Success\nerror CommitStatusError CommitStatusError is for when the CommitStatus is Error\nfailure CommitStatusFailure CommitStatusFailure is for when the CommitStatus is Failure\nwarning CommitStatusWarning CommitStatusWarning is for when the CommitStatus is Warning\nskipped CommitStatusSkipped CommitStatusSkipped is for when CommitStatus is Skipped", + "x-go-name": "State" }, "target_url": { "type": "string", From 1aa7634d9d78051f42a86281f3ce4c96454b6fb2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 28 May 2025 17:10:37 -0700 Subject: [PATCH 3/7] Fix tests --- models/git/commit_status_test.go | 6 +++--- services/pull/commit_status.go | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index f7c8e30beb573..351cafce16c9a 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -124,7 +124,7 @@ func Test_CalcCommitStatus(t *testing.T) { }, }, expected: &git_model.CommitStatus{ - State: commitstatus.CommitStatusError, + State: commitstatus.CommitStatusFailure, }, }, { @@ -140,7 +140,7 @@ func Test_CalcCommitStatus(t *testing.T) { }, }, expected: &git_model.CommitStatus{ - State: commitstatus.CommitStatusWarning, + State: commitstatus.CommitStatusSuccess, }, }, { @@ -172,7 +172,7 @@ func Test_CalcCommitStatus(t *testing.T) { }, }, expected: &git_model.CommitStatus{ - State: commitstatus.CommitStatusError, + State: commitstatus.CommitStatusFailure, }, }, } diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index ea95a2c63a78e..d3a0f718a7f9e 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -20,6 +20,10 @@ import ( // MergeRequiredContextsCommitStatus returns a commit status state for given required contexts func MergeRequiredContextsCommitStatus(commitStatuses []*git_model.CommitStatus, requiredContexts []string) commitstatus.CommitStatusState { + if len(commitStatuses) == 0 { + return commitstatus.CommitStatusPending + } + if len(requiredContexts) == 0 { return git_model.CalcCommitStatus(commitStatuses).State } From ec2cc24c4b5b59d7c3204ae8c0c5f4a5e595d454 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 28 May 2025 18:57:15 -0700 Subject: [PATCH 4/7] Fix tests --- models/git/commit_status_test.go | 2 +- routers/web/repo/commit.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index 351cafce16c9a..5225a67fb7310 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -140,7 +140,7 @@ func Test_CalcCommitStatus(t *testing.T) { }, }, expected: &git_model.CommitStatus{ - State: commitstatus.CommitStatusSuccess, + State: commitstatus.CommitStatusPending, }, }, { diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 1ea881c2a79b7..9dd6988825c2e 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -457,6 +457,9 @@ func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) ([]*git_m } if !ctx.Repo.CanRead(unit_model.TypeActions) { for _, commit := range commits { + if commit.Status == nil { + continue + } commit.Status.HideActionsURL(ctx) git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) } From 3f6ed814e765751fb798c663c48d9ed7b42fdf42 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 11:41:06 -0700 Subject: [PATCH 5/7] Fix test --- models/git/commit_status_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index 5225a67fb7310..c80fc18334447 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -178,7 +178,7 @@ func Test_CalcCommitStatus(t *testing.T) { } for _, kase := range kases { - assert.Equal(t, kase.expected, git_model.CalcCommitStatus(kase.statuses)) + assert.Equal(t, kase.expected, git_model.CalcCommitStatus(kase.statuses), fmt.Sprintf("statuses: %v", kase.statuses)) } } From ca473def5ea1892f8314e7492a1e4c2c5870bccd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 13:05:06 -0700 Subject: [PATCH 6/7] Fix test --- models/git/commit_status_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index c80fc18334447..4c0f5e891b26b 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -178,7 +178,7 @@ func Test_CalcCommitStatus(t *testing.T) { } for _, kase := range kases { - assert.Equal(t, kase.expected, git_model.CalcCommitStatus(kase.statuses), fmt.Sprintf("statuses: %v", kase.statuses)) + assert.Equal(t, kase.expected, git_model.CalcCommitStatus(kase.statuses), "statuses: %v", kase.statuses) } } From 1ad800dc2e4daed0f18fc34e793d0aa3bfb812cc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 16:22:35 -0700 Subject: [PATCH 7/7] Fix test --- routers/web/repo/issue_view.go | 3 +++ tests/integration/pull_status_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 13b9d83da42d0..c49a8cc9bc076 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -757,6 +757,9 @@ func prepareIssueViewCommentsAndSidebarParticipants(ctx *context.Context, issue } if !ctx.Repo.CanRead(unit.TypeActions) { for _, commit := range comment.Commits { + if commit.Status == nil { + continue + } commit.Status.HideActionsURL(ctx) git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) } diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 00ffcfb632c0c..bfcb97b082525 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -100,7 +100,7 @@ func TestPullCreate_CommitStatus(t *testing.T) { repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) css := unittest.AssertExistsAndLoadBean(t, &git_model.CommitStatusSummary{RepoID: repo1.ID, SHA: commitID}) - assert.Equal(t, commitstatus.CommitStatusWarning, css.State) + assert.Equal(t, commitstatus.CommitStatusSuccess, css.State) }) }