[v12.0/forgejo] fix: cancelled or skipped runs are not failures for notifications (#8404)
Some checks failed
/ release (push) Has been cancelled
testing-integration / test-unit (push) Has been cancelled
testing-integration / test-sqlite (push) Has been cancelled
testing / backend-checks (push) Has been cancelled
testing / frontend-checks (push) Has been cancelled
testing / test-unit (push) Has been cancelled
testing / test-e2e (push) Has been cancelled
testing / test-remote-cacher (redis) (push) Has been cancelled
testing / test-remote-cacher (valkey) (push) Has been cancelled
testing / test-remote-cacher (garnet) (push) Has been cancelled
testing / test-remote-cacher (redict) (push) Has been cancelled
testing / test-mysql (push) Has been cancelled
testing / test-pgsql (push) Has been cancelled
testing / test-sqlite (push) Has been cancelled
testing / security-check (push) Has been cancelled

**Backport:** https://codeberg.org/forgejo/forgejo/pulls/8366

From the point of view of a notification, the only status which must be considered a fail is `StatusFailure`. All others are either success `StatusSuccess` or undetermined `StatusCancelled` and `StatusSkipped`.

Those are the only four status in which a run can be when it reaches the `ActionRunNowDone` function because it is filtered on `IsDone` which is only true for those.

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. 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

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.

### Documentation

- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [x] I do not want this change to show in the release notes.

Co-authored-by: Earl Warren <contact@earl-warren.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8404
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
Co-committed-by: forgejo-backport-action <forgejo-backport-action@noreply.codeberg.org>
This commit is contained in:
forgejo-backport-action 2025-07-03 23:28:01 +02:00 committed by Earl Warren
parent 36777a3a9e
commit d4c9c01f2d
3 changed files with 88 additions and 3 deletions

View file

@ -16,8 +16,8 @@ const (
tplActionNowDone base.TplName = "actions/now_done"
)
// requires !run.Status.IsSuccess() or !lastRun.Status.IsSuccess()
func MailActionRun(run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error {
var MailActionRun = mailActionRun // make it mockable
func mailActionRun(run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error {
if setting.MailService == nil {
// No mail service configured
return nil

View file

@ -57,6 +57,91 @@ func assertTranslatedLocaleMailActionsNowDone(t *testing.T, msgBody string) {
AssertTranslatedLocale(t, msgBody, "mail.actions.successful_run_after_failure", "mail.actions.not_successful_run", "mail.actions.run_info_cur_status", "mail.actions.run_info_ref", "mail.actions.run_info_previous_status", "mail.actions.run_info_trigger", "mail.view_it_on")
}
func TestActionRunNowDoneStatusMatrix(t *testing.T) {
successStatuses := []actions_model.Status{
actions_model.StatusSuccess,
actions_model.StatusSkipped,
actions_model.StatusCancelled,
}
failureStatuses := []actions_model.Status{
actions_model.StatusFailure,
}
for _, testCase := range []struct {
name string
statuses []actions_model.Status
hasLastRun bool
lastStatuses []actions_model.Status
run bool
}{
{
name: "FailureNoLastRun",
statuses: failureStatuses,
run: true,
},
{
name: "SuccessNoLastRun",
statuses: successStatuses,
run: false,
},
{
name: "FailureLastRunSuccess",
statuses: failureStatuses,
hasLastRun: true,
lastStatuses: successStatuses,
run: true,
},
{
name: "FailureLastRunFailure",
statuses: failureStatuses,
hasLastRun: true,
lastStatuses: failureStatuses,
run: true,
},
{
name: "SuccessLastRunFailure",
statuses: successStatuses,
hasLastRun: true,
lastStatuses: failureStatuses,
run: true,
},
{
name: "SuccessLastRunSuccess",
statuses: successStatuses,
hasLastRun: true,
lastStatuses: successStatuses,
run: false,
},
} {
t.Run(testCase.name, func(t *testing.T) {
var called bool
defer test.MockVariableValue(&MailActionRun, func(run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error {
called = true
return nil
})()
for _, status := range testCase.statuses {
for _, lastStatus := range testCase.lastStatuses {
called = false
n := NewNotifier()
var lastRun *actions_model.ActionRun
if testCase.hasLastRun {
lastRun = &actions_model.ActionRun{
Status: lastStatus,
}
}
n.ActionRunNowDone(t.Context(),
&actions_model.ActionRun{
Status: status,
},
actions_model.StatusUnknown,
lastRun)
assert.Equal(t, testCase.run, called, "status = %s, lastStatus = %s", status, lastStatus)
}
}
})
}
}
func TestActionRunNowDoneNotificationMail(t *testing.T) {
ctx := t.Context()

View file

@ -212,7 +212,7 @@ func (m *mailNotifier) NewUserSignUp(ctx context.Context, newUser *user_model.Us
func (m *mailNotifier) ActionRunNowDone(ctx context.Context, run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) {
// Only send a mail on a successful run when the workflow recovered (i.e., the run before failed).
if run.Status.IsSuccess() && (lastRun == nil || lastRun.Status.IsSuccess()) {
if !run.Status.IsFailure() && (lastRun == nil || !lastRun.Status.IsFailure()) {
return
}
if err := MailActionRun(run, priorStatus, lastRun); err != nil {