From 4747dd68bdc979144021d0d9e666e56dbca4f191 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 31 Mar 2026 19:22:18 +0200 Subject: [PATCH] Update Combine method to treat warnings as failures and adjust tests (#37048) Treat Commit Status Warnings as errors > The root problem is that the definition of "warning" are different across systems. > > * Sometimes, "warning" is treated as "acceptable" (Gitea 1.25) > * Sometimes, "warning" is mapped from "Result.UNSTABLE", which means "there are test failures" and it is "failure" in Gitea > > **To avoid breaking existing users, the best choice is to revert the behavior on Gitea side: treat "warning" as "error".** https://github.com/go-gitea/gitea/issues/37042#issuecomment-4158231611 fixes https://github.com/go-gitea/gitea/issues/37042 --------- Signed-off-by: Nicolas Co-authored-by: wxiaoguang --- models/git/commit_status_test.go | 2 +- modules/commitstatus/commit_status.go | 5 +++-- modules/commitstatus/commit_status_test.go | 14 +++++++------- tests/integration/pull_status_test.go | 2 +- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index f565550c53..f18d3470b0 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -139,7 +139,7 @@ func Test_CalcCommitStatus(t *testing.T) { }, }, expected: &git_model.CommitStatus{ - State: commitstatus.CommitStatusPending, + State: commitstatus.CommitStatusFailure, }, }, { diff --git a/modules/commitstatus/commit_status.go b/modules/commitstatus/commit_status.go index a0ab4e7186..9c6994249b 100644 --- a/modules/commitstatus/commit_status.go +++ b/modules/commitstatus/commit_status.go @@ -61,16 +61,17 @@ type CommitStatusStates []CommitStatusState //nolint:revive // export stutter // 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 +// > failure if any of the contexts report as warning (Gitea specific behavior) // > 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(): + case state.IsError() || state.IsFailure() || state.IsWarning(): return CommitStatusFailure case state.IsPending(): - case state.IsSuccess() || state.IsWarning() || state.IsSkipped(): + case state.IsSuccess() || state.IsSkipped(): successCnt++ } } diff --git a/modules/commitstatus/commit_status_test.go b/modules/commitstatus/commit_status_test.go index 10d8f20aa4..dc8f9ce1a5 100644 --- a/modules/commitstatus/commit_status_test.go +++ b/modules/commitstatus/commit_status_test.go @@ -41,7 +41,7 @@ func TestCombine(t *testing.T) { { name: "warning", states: CommitStatusStates{CommitStatusWarning}, - expected: CommitStatusSuccess, + expected: CommitStatusFailure, }, // 2 states { @@ -62,7 +62,7 @@ func TestCombine(t *testing.T) { { name: "pending and warning", states: CommitStatusStates{CommitStatusPending, CommitStatusWarning}, - expected: CommitStatusPending, + expected: CommitStatusFailure, }, { name: "success and error", @@ -77,7 +77,7 @@ func TestCombine(t *testing.T) { { name: "success and warning", states: CommitStatusStates{CommitStatusSuccess, CommitStatusWarning}, - expected: CommitStatusSuccess, + expected: CommitStatusFailure, }, { name: "error and failure", @@ -98,7 +98,7 @@ func TestCombine(t *testing.T) { { name: "pending, success and warning", states: CommitStatusStates{CommitStatusPending, CommitStatusSuccess, CommitStatusWarning}, - expected: CommitStatusPending, + expected: CommitStatusFailure, }, { name: "pending, success and error", @@ -133,7 +133,7 @@ func TestCombine(t *testing.T) { { name: "success, warning and skipped", states: CommitStatusStates{CommitStatusSuccess, CommitStatusWarning, CommitStatusSkipped}, - expected: CommitStatusSuccess, + expected: CommitStatusFailure, }, // All success { @@ -181,12 +181,12 @@ func TestCombine(t *testing.T) { { name: "mixed states with all success", states: CommitStatusStates{CommitStatusSuccess, CommitStatusSuccess, CommitStatusPending, CommitStatusWarning}, - expected: CommitStatusPending, + expected: CommitStatusFailure, }, { name: "all success with warning", states: CommitStatusStates{CommitStatusSuccess, CommitStatusSuccess, CommitStatusSuccess, CommitStatusWarning}, - expected: CommitStatusSuccess, + expected: CommitStatusFailure, }, } diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index eb57943017..3b8924b6e6 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -59,8 +59,8 @@ func TestPullCreate_CommitStatus(t *testing.T) { commitstatus.CommitStatusPending, commitstatus.CommitStatusError, commitstatus.CommitStatusFailure, - commitstatus.CommitStatusSuccess, commitstatus.CommitStatusWarning, + commitstatus.CommitStatusSuccess, } statesIcons := map[commitstatus.CommitStatusState]string{