From 8a1021e2a08917b9ce8f2c286c09baa4a7a2887f Mon Sep 17 00:00:00 2001 From: Andreas Ahlenstorf Date: Sun, 17 May 2026 18:00:49 +0200 Subject: [PATCH] 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/.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 Reviewed-by: Mathieu Fenniak --- models/fixtures/commit_status.yml | 12 +++++++ models/git/commit_status_test.go | 33 +++++++++++++++-- modules/structs/commit_status.go | 12 +++++-- modules/structs/commit_status_test.go | 40 +++++++++++++++++++++ options/locale_next/locale_en-US.json | 1 + services/actions/commit_status.go | 4 ++- templates/repo/commit_status.tmpl | 3 ++ templates/repo/pulls/status.tmpl | 2 ++ templates/swagger/v1_json.tmpl | 2 +- tests/integration/commit_status_test.go | 24 +++++++++++++ tests/integration/pull_status_test.go | 2 ++ web_src/js/components/DashboardRepoList.vue | 1 + 12 files changed, 129 insertions(+), 7 deletions(-) diff --git a/models/fixtures/commit_status.yml b/models/fixtures/commit_status.yml index c568e89cea..815501bd69 100644 --- a/models/fixtures/commit_status.yml +++ b/models/fixtures/commit_status.yml @@ -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 diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index b5c3690e0f..4b652b401d 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -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, diff --git a/modules/structs/commit_status.go b/modules/structs/commit_status.go index 2a5dfb7546..e2d6353cc4 100644 --- a/modules/structs/commit_status.go +++ b/modules/structs/commit_status.go @@ -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 +} diff --git a/modules/structs/commit_status_test.go b/modules/structs/commit_status_test.go index f06808534c..5d983c7cc4 100644 --- a/modules/structs/commit_status_test.go +++ b/modules/structs/commit_status_test.go @@ -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{ diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index f8ddb6f59c..5ed2c48152 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -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", diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 2a8ca2810c..75bff9975b 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -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: diff --git a/templates/repo/commit_status.tmpl b/templates/repo/commit_status.tmpl index eb700ab2bb..28f5fae66c 100644 --- a/templates/repo/commit_status.tmpl +++ b/templates/repo/commit_status.tmpl @@ -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}} diff --git a/templates/repo/pulls/status.tmpl b/templates/repo/pulls/status.tmpl index e8636ba1b8..56e8259f4e 100644 --- a/templates/repo/pulls/status.tmpl +++ b/templates/repo/pulls/status.tmpl @@ -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"}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fa9750db72..66a1f01515 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -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" }, diff --git a/tests/integration/commit_status_test.go b/tests/integration/commit_status_test.go index adcedcc41b..b4b4a9a8bc 100644 --- a/tests/integration/commit_status_test.go +++ b/tests/integration/commit_status_test.go @@ -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) }) diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index f790a09c44..844fcab3c0 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -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", diff --git a/web_src/js/components/DashboardRepoList.vue b/web_src/js/components/DashboardRepoList.vue index 3dcfeb2c73..0f5e44e787 100644 --- a/web_src/js/components/DashboardRepoList.vue +++ b/web_src/js/components/DashboardRepoList.vue @@ -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'},