feat: mark skipped checks as skipped (#12606)

A separate commit status is introduced for skipped checks. That enables marking them as such in the UI instead of successful, which could be misleading.

Resolves https://codeberg.org/forgejo/forgejo/issues/10138.

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests for Go changes

(can be removed for JavaScript changes)

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.
  - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I ran...
  - [x] `make pr-go` before pushing

### Tests for JavaScript changes

(can be removed for Go changes)

- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

*The decision if the pull request will be shown in the release notes is up to the mergers / release team.*

The content of the `release-notes/<pull request number>.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12606
Reviewed-by: Cyborus <cyborus@disroot.org>
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
This commit is contained in:
Andreas Ahlenstorf 2026-05-17 18:00:49 +02:00 committed by Mathieu Fenniak
parent 4ecb25a549
commit 8a1021e2a0
12 changed files with 129 additions and 7 deletions

View file

@ -93,3 +93,15 @@
context: deploy/awesomeness
context_hash: ae9547713a6665fc4261d0756904932085a41cf2
creator_id: 2
-
id: 9
index: 7
repo_id: 1
state: "skipped"
sha: "1234123412341234123412341234123412341234"
target_url: https://example.com/builds/
description: Publish awesomeness
context: publish/awesomeness
context_hash: 701785a796917a4942f2ab1337d95225270026b5
creator_id: 2

View file

@ -33,8 +33,8 @@ func TestGetCommitStatuses(t *testing.T) {
SHA: sha1,
})
require.NoError(t, err)
assert.EqualValues(t, 6, maxResults)
assert.Len(t, statuses, 6)
assert.EqualValues(t, 7, maxResults)
assert.Len(t, statuses, 7)
assert.Equal(t, "ci/awesomeness", statuses[0].Context)
assert.Equal(t, structs.CommitStatusPending, statuses[0].State)
@ -60,13 +60,17 @@ func TestGetCommitStatuses(t *testing.T) {
assert.Equal(t, structs.CommitStatusPending, statuses[5].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[5].APIURL(db.DefaultContext))
assert.Equal(t, "publish/awesomeness", statuses[6].Context)
assert.Equal(t, structs.CommitStatusSkipped, statuses[6].State)
assert.Equal(t, "https://try.gitea.io/api/v1/repos/user2/repo1/statuses/1234123412341234123412341234123412341234", statuses[6].APIURL(db.DefaultContext))
statuses, maxResults, err = db.FindAndCount[git_model.CommitStatus](db.DefaultContext, &git_model.CommitStatusOptions{
ListOptions: db.ListOptions{Page: 2, PageSize: 50},
RepoID: repo1.ID,
SHA: sha1,
})
require.NoError(t, err)
assert.EqualValues(t, 6, maxResults)
assert.EqualValues(t, 7, maxResults)
assert.Empty(t, statuses)
}
@ -182,6 +186,29 @@ func Test_CalcCommitStatus(t *testing.T) {
State: structs.CommitStatusError,
},
},
{
statuses: []*git_model.CommitStatus{
{
State: structs.CommitStatusSkipped,
},
},
expected: &git_model.CommitStatus{
State: structs.CommitStatusSkipped,
},
},
{
statuses: []*git_model.CommitStatus{
{
State: structs.CommitStatusSuccess,
},
{
State: structs.CommitStatusSkipped,
},
},
expected: &git_model.CommitStatus{
State: structs.CommitStatusSuccess,
},
},
{
statuses: []*git_model.CommitStatus{},
expected: nil,

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", "failure", "warning", or "skipped"
type CommitStatusState string
const (
@ -18,6 +18,8 @@ const (
CommitStatusFailure CommitStatusState = "failure"
// CommitStatusWarning is for when the CommitStatus is Warning
CommitStatusWarning CommitStatusState = "warning"
// CommitStatusSkipped is for when the CommitStatus is Skipped
CommitStatusSkipped CommitStatusState = "skipped"
)
var commitStatusPriorities = map[CommitStatusState]int{
@ -26,6 +28,7 @@ var commitStatusPriorities = map[CommitStatusState]int{
CommitStatusWarning: 2,
CommitStatusPending: 3,
CommitStatusSuccess: 4,
CommitStatusSkipped: 5,
}
func (css CommitStatusState) String() string {
@ -35,7 +38,7 @@ func (css CommitStatusState) String() string {
// 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
// NoBetterThan only handles the states above
if _, exist := commitStatusPriorities[css]; !exist {
return false
}
@ -71,3 +74,8 @@ func (css CommitStatusState) IsFailure() bool {
func (css CommitStatusState) IsWarning() bool {
return css == CommitStatusWarning
}
// IsSkipped returns true if a commit has been skipped.
func (css CommitStatusState) IsSkipped() bool {
return css == CommitStatusSkipped
}

View file

@ -18,6 +18,46 @@ func TestNoBetterThan(t *testing.T) {
args args
want bool
}{
{
name: "skipped is not better than skipped",
args: args{
css: CommitStatusSkipped,
css2: CommitStatusSkipped,
},
want: true,
},
{
name: "skipped is not better than success",
args: args{
css: CommitStatusSkipped,
css2: CommitStatusSuccess,
},
want: false,
},
{
name: "skipped is not better than pending",
args: args{
css: CommitStatusSkipped,
css2: CommitStatusPending,
},
want: false,
},
{
name: "skipped is not better than failure",
args: args{
css: CommitStatusSkipped,
css2: CommitStatusFailure,
},
want: false,
},
{
name: "skipped is not better than error",
args: args{
css: CommitStatusSkipped,
css2: CommitStatusError,
},
want: false,
},
{
name: "success is no better than success",
args: args{

View file

@ -90,6 +90,7 @@
},
"repo.pulls.maintainers_can_edit": "Maintainers can edit this pull request.",
"repo.pulls.maintainers_cannot_edit": "Maintainers cannot edit this pull request.",
"repo.pulls.status_checks_skipped": "Skipped",
"repo.form.cannot_create": "All spaces in which you can create repositories have reached the limit of repositories.",
"repo.view.gitmodules_too_large": "The .gitmodules file is too large and will be ignored (on API calls for instance)",
"migrate.select.title": "Migrate repository",

View file

@ -152,8 +152,10 @@ 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.StatusSkipped:
return api.CommitStatusSkipped
case actions_model.StatusFailure, actions_model.StatusCancelled:
return api.CommitStatusFailure
case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning:

View file

@ -5,6 +5,9 @@
{{if eq .State "success"}}
{{svg "octicon-check" 18 "commit-status icon text green"}}
{{end}}
{{if eq .State "skipped"}}
{{svg "octicon-skip" 18 "commit-status icon text grey"}}
{{end}}
{{if eq .State "error"}}
{{svg "gitea-exclamation" 18 "commit-status icon text red"}}
{{end}}

View file

@ -14,6 +14,8 @@ Template Attributes:
{{ctx.Locale.Tr "repo.pulls.status_checking"}}
{{else if eq .CommitStatus.State "success"}}
{{ctx.Locale.Tr "repo.pulls.status_checks_success"}}
{{else if eq .CommitStatus.State "skipped"}}
{{ctx.Locale.Tr "repo.pulls.status_checks_skipped"}}
{{else if eq .CommitStatus.State "warning"}}
{{ctx.Locale.Tr "repo.pulls.status_checks_warning"}}
{{else if eq .CommitStatus.State "failure"}}

View file

@ -24114,7 +24114,7 @@
"x-go-package": "forgejo.org/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\", \"failure\", \"warning\", or \"skipped\"",
"type": "string",
"x-go-package": "forgejo.org/modules/structs"
},

View file

@ -33,6 +33,18 @@ func TestGetLatestCommitStatusForPairs(t *testing.T) {
assert.Equal(t, map[int64][]*git_model.CommitStatus{
1: {
{
ID: 9,
Index: 7,
RepoID: 1,
State: structs.CommitStatusSkipped,
SHA: "1234123412341234123412341234123412341234",
TargetURL: "https://example.com/builds/",
Description: "Publish awesomeness",
ContextHash: "701785a796917a4942f2ab1337d95225270026b5",
Context: "publish/awesomeness",
CreatorID: 2,
},
{
ID: 7,
Index: 6,
@ -164,6 +176,18 @@ func TestGetLatestCommitStatusForRepoCommitIDs(t *testing.T) {
Context: "deploy/awesomeness",
CreatorID: 2,
},
{
ID: 9,
Index: 7,
RepoID: 1,
State: structs.CommitStatusSkipped,
SHA: "1234123412341234123412341234123412341234",
TargetURL: "https://example.com/builds/",
Description: "Publish awesomeness",
ContextHash: "701785a796917a4942f2ab1337d95225270026b5",
Context: "publish/awesomeness",
CreatorID: 2,
},
},
}, repoStatuses)
})

View file

@ -55,12 +55,14 @@ func TestPullCreate_CommitStatus(t *testing.T) {
api.CommitStatusError,
api.CommitStatusFailure,
api.CommitStatusSuccess,
api.CommitStatusSkipped,
api.CommitStatusWarning,
}
statesIcons := map[api.CommitStatusState]string{
api.CommitStatusPending: "octicon-dot-fill",
api.CommitStatusSuccess: "octicon-check",
api.CommitStatusSkipped: "octicon-skip",
api.CommitStatusError: "gitea-exclamation",
api.CommitStatusFailure: "octicon-x",
api.CommitStatusWarning: "gitea-exclamation",

View file

@ -9,6 +9,7 @@ const {appSubUrl, assetUrlPrefix, pageData} = window.config;
const commitStatus = {
pending: {name: 'octicon-dot-fill', color: 'yellow'},
success: {name: 'octicon-check', color: 'green'},
skipped: {name: 'octicon-skip', color: 'grey'},
error: {name: 'gitea-exclamation', color: 'red'},
failure: {name: 'octicon-x', color: 'red'},
warning: {name: 'gitea-exclamation', color: 'yellow'},