Incorrect aggregated status of commit statuses

[harryzcy]

Description

The commit as part of PR is marked with green check, when some workflows is still running.

Gitea Version

v1.19.4

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

Git Version

No response

Operating System

No response

How are you running Gitea?

official docker image

Database

PostgreSQL

[wolfogre]

I don't think it's related to Actions, Actions create commit statuses, and it seems that the aggregated status is incorrect.

[sebastian-sauer]

What is the current "order" / priority for those states?
If the expected behaviour is clear i'm happy to write a test & fix for this.

@lunny you did the last implementation of this order - so maybe you got any thoughts on this?

I'd assume the following :

Order (always if one state is available first, then this should be the combined state)

[Pending, Running, Error, Failure, Warning, Success]

so if one is pending -> show pending
if none is pending but 1 is running -> show pending
and so on :)

As far as i understand this aligns with the github documentation (which is not very detailled ;))

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

This would change the evaluation - please see results compared:

Method	Old-result	New-result
pending.NoBetterThan(pending)	true	true
pending.NoBetterThan(running)	true	true
pending.NoBetterThan(error)	false	true
pending.NoBetterThan(failure)	false	true
pending.NoBetterThan(warning)	false	true
pending.NoBetterThan(success)	true	true
running.NoBetterThan(pending)	false	false
running.NoBetterThan(running)	true	true
running.NoBetterThan(error)	false	true
running.NoBetterThan(failure)	false	true
running.NoBetterThan(warning)	false	true
running.NoBetterThan(success)	true	true
error.NoBetterThan(pending)	true	false
error.NoBetterThan(running)	true	false
error.NoBetterThan(error)	true	true
error.NoBetterThan(failure)	true	true
error.NoBetterThan(warning)	true	true
error.NoBetterThan(success)	true	true
failure.NoBetterThan(pending)	true	false
failure.NoBetterThan(running)	true	false
failure.NoBetterThan(error)	false	false
failure.NoBetterThan(failure)	true	true
failure.NoBetterThan(warning)	true	true
failure.NoBetterThan(success)	true	true
warning.NoBetterThan(pending)	true	false
warning.NoBetterThan(running)	true	false
warning.NoBetterThan(error)	false	false
warning.NoBetterThan(failure)	false	false
warning.NoBetterThan(warning)	true	true
warning.NoBetterThan(success)	true	true
success.NoBetterThan(pending)	false	false
success.NoBetterThan(running)	true	false
success.NoBetterThan(error)	false	false
success.NoBetterThan(failure)	false	false
success.NoBetterThan(warning)	false	false
success.NoBetterThan(success)	true	true

[CaiCandong]

gitea/modules/structs/commit_status.go

Lines 26 to 40 in 491cc06

	func (css CommitStatusState) NoBetterThan(css2 CommitStatusState) bool {
	switch css {
	case CommitStatusError:
	return true
	case CommitStatusFailure:
	return css2 != CommitStatusError
	case CommitStatusWarning:
	return css2 != CommitStatusError && css2 != CommitStatusFailure
	case CommitStatusPending:
	return css2 != CommitStatusError && css2 != CommitStatusFailure && css2 != CommitStatusWarning
	default:
	return css2 != CommitStatusError && css2 != CommitStatusFailure && css2 != CommitStatusWarning && css2 != CommitStatusPending
	}
	}

The truth table for the above function is as follows ， CommitStatusSuccess <=CommitStatusRunning should be false?

NoBetterThan(<=)	CommitStatusError	CommitStatusFailure	CommitStatusWarning	CommitStatusPending	CommitStatusRunning	CommitStatusSuccess
CommitStatusError	true	true	true	true	true	true
CommitStatusFailure	false	true	true	true	true	true
CommitStatusWarning	false	false	true	true	true	true
CommitStatusPending	false	false	false	true	true	true
CommitStatusRunning	false	false	false	false	true	true
CommitStatusSuccess	false	false	false	false	true	true

[CaiCandong]

1. For this issue solution, I noticed that the priority in the source code is [Error, Failure, Warning, Pending,Running,Success], but there might be a little problem in the implementation, which leads to Success.NoBetterThan(Running) return true, we just need to fix this little problem to resolve the issue.

2. Regarding the need to align with github, @sebastian-sauer you mentioned the github implementation:.

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

But I noticed that there are only 4 statuses for github create-a-commit-status

state string Required
The state of the status.

Can be one of: error, failure, pending, success

The design of gitea is still somewhat different from github, so we can't fully align it with github.

3. I don't quite understand why @sebastian-sauer's priority is [Pending, Running, Error, Failure, Warning, Success], according to github's documentation, I think github's priority should be [error,failure, Pending,Success], and either error or failure returns failure.This contradicts your suggestion of [Pending, Running, Error, Failure, Warning, Success], when both Running and Failure are present the , the combined state should be Failure, right? But depending on your priority, it will return Running.

[wolfogre]

My idea is to remove running as a commit status. It was added because Gitea Actions has running, but it could be a mistake. I think pending is enough to express the waiting, running or blocked for Gitea Actions.

And warning may be also a mistake too, I have no idea when to use warning instead of failure/error.