From 4ca6b703af64cc9bdb457e827a5d16f683c2b052 Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Sat, 4 Apr 2026 19:16:35 +0200 Subject: [PATCH] [v15.0/forgejo] feat: support `timezone` in scheduled workflows (#11986) **Backport:** https://codeberg.org/forgejo/forgejo/pulls/11851 GitHub recently added the ability to [specify a time zone for scheduled workflows](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#onschedule), thereby making it possible to run scheduled workflows at a certain local time, no matter whether daylight saving time (DST) is currently active or not. Example copied from GitHub's documentation: ```yaml on: schedule: - cron: '30 5 * * 1-5' timezone: "America/New_York" ``` The workflow would run at 05:30 each morning in the America/New_York timezone every Monday through Friday. `timezone` accepts IANA time zone names. If `timezone` is absent, `Etc/UTC` is used. GitHub runs workflows that were scheduled during DST jumps forward, for example, between 2 o'clock and 3 o'clock, directly after the clock jumped forward. In this case, that would be 3 o'clock. Forgejo already supports time zones by prepending cron schedules with `TZ=` or `CRON_TZ=`: ```yaml on: schedule: - cron: 'CRON_TZ=America/New_York 30 5 * * 1-5' ``` However, that capability is not documented. Workflows that are scheduled to run during DST changes are skipped when the clock jumps forward and run twice when it jumps backward. This two-part PR adds support for `timezone` to improve compatibility with GitHub. `TZ` and `CRON_TZ` continue working. When both `timezone` and `TZ` or `CRON_TZ` are present, `timezone` takes precedence. When neither `timezone` nor `TZ` nor `CRON_TZ` are present, `Etc/UTC` is used as before. Because `TZ` and `CRON_TZ` were already supported by Forgejo before GitHub introduced `timezone`, `timezone` behaves during DST changes as previous versions of Forgejo, thereby deviating from GitHub. That means that workflows that are scheduled to run during DST changes are skipped when the clock jumps forward. And they run twice when it jumps backwards. However, it is generally recommended not to schedule workflows during the time of day when DST changes occur. This part of the PR integrates the [workflow validation and parsing of the `timezone` field](https://code.forgejo.org/forgejo/runner/pulls/1454) supplied by Forgejo Runner. ## 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 - [x] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - https://codeberg.org/forgejo/docs/pulls/1853 - [ ] 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. ## Release notes - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/11851): support `timezone` in scheduled workflows Co-authored-by: Andreas Ahlenstorf Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11986 Reviewed-by: Mathieu Fenniak Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- go.mod | 2 +- go.sum | 4 +- models/actions/schedule.go | 22 +--- models/actions/schedule_spec.go | 44 ++++++-- models/actions/schedule_spec_test.go | 79 ++++++++++++-- models/actions/schedule_test.go | 102 ++++++++++++++++++ .../v15c_add_schedule_spec_time_zones.go | 31 ++++++ .../action_schedule.yml | 4 - services/actions/notifier_helper.go | 14 ++- services/actions/schedule_tasks_test.go | 30 ++++-- tests/integration/actions_trigger_test.go | 49 ++++++++- 11 files changed, 323 insertions(+), 58 deletions(-) create mode 100644 models/actions/schedule_test.go create mode 100644 models/forgejo_migrations/v15c_add_schedule_spec_time_zones.go diff --git a/go.mod b/go.mod index 9f751ccbb1..b37a7eb424 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( code.forgejo.org/forgejo/go-rpmutils v1.0.0 code.forgejo.org/forgejo/levelqueue v1.0.0 code.forgejo.org/forgejo/reply v1.0.2 - code.forgejo.org/forgejo/runner/v12 v12.7.3 + code.forgejo.org/forgejo/runner/v12 v12.8.0 code.forgejo.org/go-chi/binding v1.0.1 code.forgejo.org/go-chi/cache v1.0.1 code.forgejo.org/go-chi/captcha v1.0.2 diff --git a/go.sum b/go.sum index 3e4ee526b9..76e13223a8 100644 --- a/go.sum +++ b/go.sum @@ -30,8 +30,8 @@ code.forgejo.org/forgejo/levelqueue v1.0.0 h1:9krYpU6BM+j/1Ntj6m+VCAIu0UNnne1/Uf code.forgejo.org/forgejo/levelqueue v1.0.0/go.mod h1:fmG6zhVuqim2rxSFOoasgXO8V2W/k9U31VVYqLIRLhQ= code.forgejo.org/forgejo/reply v1.0.2 h1:dMhQCHV6/O3L5CLWNTol+dNzDAuyCK88z4J/lCdgFuQ= code.forgejo.org/forgejo/reply v1.0.2/go.mod h1:RyZUfzQLc+fuLIGjTSQWDAJWPiL4WtKXB/FifT5fM7U= -code.forgejo.org/forgejo/runner/v12 v12.7.3 h1:+thSawVfLeAZaWB6sYeUPvLj4lxYjCIDt/ktvkfX5Rs= -code.forgejo.org/forgejo/runner/v12 v12.7.3/go.mod h1:OO+Vy9Dww6WNV7GG/6VUWo/0WwXY+ASGlINmAfEA9Ws= +code.forgejo.org/forgejo/runner/v12 v12.8.0 h1:/MqOseYbsGaQ2qzepaZr3VyuqpESvSP/ZnC2aKfmU3g= +code.forgejo.org/forgejo/runner/v12 v12.8.0/go.mod h1:sgDAYfO4NJI1kUzGuD7klHuoFLQzWmZPw0erg7QlbJU= code.forgejo.org/forgejo/ssh v0.0.0-20241211213324-5fc306ca0616 h1:kEZL84+02jY9RxXM4zHBWZ3Fml0B09cmP1LGkDsCfIA= code.forgejo.org/forgejo/ssh v0.0.0-20241211213324-5fc306ca0616/go.mod h1:zpHEXBstFnQYtGnB8k8kQLol82umzn/2/snG7alWVD8= code.forgejo.org/go-chi/binding v1.0.1 h1:coKNI+X1NzRN7X85LlrpvBRqk0TXpJ+ja28vusQWEuY= diff --git a/models/actions/schedule.go b/models/actions/schedule.go index 05c9f15d38..8c410b9d38 100644 --- a/models/actions/schedule.go +++ b/models/actions/schedule.go @@ -5,7 +5,6 @@ package actions import ( "context" - "time" "forgejo.org/models/db" repo_model "forgejo.org/models/repo" @@ -21,7 +20,7 @@ import ( type ActionSchedule struct { ID int64 Title string - Specs []string + Specs []*ActionScheduleSpec `xorm:"-"` RepoID int64 `xorm:"index"` Repo *repo_model.Repository `xorm:"-"` OwnerID int64 `xorm:"index"` @@ -73,25 +72,12 @@ func CreateScheduleTask(ctx context.Context, rows []*ActionSchedule) error { return err } - // Loop through each schedule spec and create a new spec row - now := time.Now() - for _, spec := range row.Specs { - specRow := &ActionScheduleSpec{ - RepoID: row.RepoID, - ScheduleID: row.ID, - Spec: spec, - } - // Parse the spec and check for errors - schedule, err := specRow.Parse() - if err != nil { - continue // skip to the next spec if there's an error - } - - specRow.Next = timeutil.TimeStamp(schedule.Next(now).Unix()) + spec.ScheduleID = row.ID + spec.RepoID = row.RepoID // Insert the new schedule spec row - if err = db.Insert(ctx, specRow); err != nil { + if err = db.Insert(ctx, spec); err != nil { return err } } diff --git a/models/actions/schedule_spec.go b/models/actions/schedule_spec.go index 83bdceb850..bcaee8bd6f 100644 --- a/models/actions/schedule_spec.go +++ b/models/actions/schedule_spec.go @@ -10,6 +10,7 @@ import ( "forgejo.org/models/db" repo_model "forgejo.org/models/repo" + "forgejo.org/modules/optional" "forgejo.org/modules/timeutil" "github.com/robfig/cron/v3" @@ -27,13 +28,28 @@ type ActionScheduleSpec struct { // started or this entry's schedule is unsatisfiable Next timeutil.TimeStamp `xorm:"index"` // Prev is the last time this job was run, or the zero time if never. - Prev timeutil.TimeStamp - Spec string + Prev timeutil.TimeStamp + Spec string + TimeZone optional.Option[string] Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` } +func NewActionScheduleSpec(cron string, tz optional.Option[string], referenceTime time.Time) (*ActionScheduleSpec, error) { + spec := &ActionScheduleSpec{ + Spec: cron, + TimeZone: tz, + } + cronSchedule, err := spec.Parse() + if err != nil { + return nil, err + } + + spec.Next = timeutil.TimeStamp(cronSchedule.Next(referenceTime).Unix()) + return spec, nil +} + // Parse parses the spec and returns a cron.Schedule // Unlike the default cron parser, Parse uses UTC timezone as the default if none is specified. func (s *ActionScheduleSpec) Parse() (cron.Schedule, error) { @@ -43,19 +59,29 @@ func (s *ActionScheduleSpec) Parse() (cron.Schedule, error) { return nil, err } - // If the spec has specified a timezone, use it - if strings.HasPrefix(s.Spec, "TZ=") || strings.HasPrefix(s.Spec, "CRON_TZ=") { - return schedule, nil - } - specSchedule, ok := schedule.(*cron.SpecSchedule) // If it's not a spec schedule, like "@every 5m", timezone is not relevant if !ok { return schedule, nil } - // Set the timezone to UTC - specSchedule.Location = time.UTC + // If `timezone` is not defined in the workflow, but the spec includes a timezone, use it. + if !s.TimeZone.Has() && (strings.HasPrefix(s.Spec, "TZ=") || strings.HasPrefix(s.Spec, "CRON_TZ=")) { + return schedule, nil + } + + var location *time.Location + if present, tz := s.TimeZone.Get(); present { + location, err = time.LoadLocation(tz) + if err != nil { + return nil, err + } + } else { + // UTC is the default time zone. + location = time.UTC + } + + specSchedule.Location = location return specSchedule, nil } diff --git a/models/actions/schedule_spec_test.go b/models/actions/schedule_spec_test.go index 0c26fce4b2..eb3a83d0a6 100644 --- a/models/actions/schedule_spec_test.go +++ b/models/actions/schedule_spec_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "forgejo.org/modules/optional" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,50 +23,105 @@ func TestActionScheduleSpec_Parse(t *testing.T) { }() time.Local = tz - now, err := time.Parse(time.RFC3339, "2024-07-31T15:47:55+08:00") - require.NoError(t, err) - tests := []struct { - name string - spec string - want string - wantErr assert.ErrorAssertionFunc + name string + refTime time.Time + spec string + timeZone string + want string + wantErr assert.ErrorAssertionFunc }{ { name: "regular", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), spec: "0 10 * * *", want: "2024-07-31T10:00:00Z", wantErr: assert.NoError, }, { name: "invalid", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), spec: "0 10 * *", want: "", wantErr: assert.Error, }, { - name: "with timezone", + name: "with TZ in cron schedule", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), spec: "TZ=America/New_York 0 10 * * *", want: "2024-07-31T14:00:00Z", wantErr: assert.NoError, }, { - name: "timezone irrelevant", + name: "with CRON_TZ in cron schedule", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), + spec: "CRON_TZ=America/New_York 0 10 * * *", + want: "2024-07-31T14:00:00Z", + wantErr: assert.NoError, + }, + { + name: "with separate time zone", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), + spec: "0 10 * * *", + timeZone: "America/New_York", + want: "2024-07-31T14:00:00Z", + wantErr: assert.NoError, + }, + { + name: "separate time zone takes precedence over inlined time zone", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), + spec: "CRON_TZ=Europe/Berlin 0 10 * * *", + timeZone: "America/New_York", + want: "2024-07-31T14:00:00Z", + wantErr: assert.NoError, + }, + { + name: "time zone irrelevant", + refTime: time.Date(2024, 7, 31, 15, 47, 55, 0, time.Local), spec: "@every 5m", want: "2024-07-31T07:52:55Z", wantErr: assert.NoError, }, + { + // The various cron implementations handle the DST jump forwards differently. The most popular approaches + // are (a) scheduling all jobs at 3 o'clock that were supposed to run between 2 and 3 o'clock, or (b) + // skipping the execution on that day because any time between 2 and 3 o'clock never happened. Forgejo uses + // option B because the code it inherited already did that and was exposed to users. + name: "skips execution during DST jump forwards", + refTime: time.Date(2025, 3, 30, 1, 5, 0, 0, time.UTC), + spec: "10 2 * * *", // The clock jumps at 2 o'clock to 3 o'clock. + timeZone: "Europe/Berlin", + want: "2025-03-31T00:10:00Z", + wantErr: assert.NoError, + }, + { + name: "executes a first time before DST jump backwards", + refTime: time.Date(2025, 10, 26, 0, 5, 0, 0, time.UTC), + spec: "10 2 * * *", // The clock jumps at 3 o'clock to 2 o'clock. + timeZone: "Europe/Berlin", + want: "2025-10-26T00:10:00Z", + wantErr: assert.NoError, + }, + { + name: "executes a second time after DST jump backwards", + refTime: time.Date(2025, 10, 26, 1, 5, 0, 0, time.UTC), + spec: "10 2 * * *", // The clock jumps at 3 o'clock to 2 o'clock. + timeZone: "Europe/Berlin", + want: "2025-10-26T01:10:00Z", + wantErr: assert.NoError, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &ActionScheduleSpec{ - Spec: tt.spec, + Spec: tt.spec, + TimeZone: optional.FromNonDefault(tt.timeZone), } got, err := s.Parse() tt.wantErr(t, err) if err == nil { - assert.Equal(t, tt.want, got.Next(now).UTC().Format(time.RFC3339)) + assert.Equal(t, tt.want, got.Next(tt.refTime).UTC().Format(time.RFC3339)) } }) } diff --git a/models/actions/schedule_test.go b/models/actions/schedule_test.go new file mode 100644 index 0000000000..016185cb42 --- /dev/null +++ b/models/actions/schedule_test.go @@ -0,0 +1,102 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + "time" + + "forgejo.org/models/db" + "forgejo.org/models/repo" + "forgejo.org/models/unittest" + "forgejo.org/models/user" + "forgejo.org/modules/optional" + "forgejo.org/modules/timeutil" + "forgejo.org/modules/webhook" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestScheduleCreateScheduleTask(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + user2 := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 2}) + repo62 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 62, Name: "test_workflows", OwnerID: user2.ID}) + + content := ` +on: + push: + schedule: + - cron: "2 13 * * *" + - cron: "03 13 * * *" + timezone: Europe/Paris +jobs: + test: + runs-on: debian + steps: + - run: | + echo "OK" +` + + referenceTime := time.Date(2026, 3, 27, 17, 41, 21, 0, time.UTC) + + specWithoutTZ, err := NewActionScheduleSpec("2 13 * * *", optional.None[string](), referenceTime) + require.NoError(t, err) + + specWithTZ, err := NewActionScheduleSpec("3 13 * * *", optional.Some("Europe/Paris"), referenceTime) + require.NoError(t, err) + + schedule := &ActionSchedule{ + Title: ".forgejo/workflows/test.yaml", + Specs: []*ActionScheduleSpec{specWithoutTZ, specWithTZ}, + RepoID: repo62.ID, + OwnerID: user2.ID, + WorkflowID: "test.yaml", + WorkflowDirectory: ".forgejo/workflows", + TriggerUserID: -2, + Ref: "main", + CommitSHA: "6af834a5bc97c1a337eb3a21d26903c5cdceca0c", + Event: webhook.HookEventPush, + EventPayload: "{\"action\":\"schedule\"}", + Content: []byte(content), + } + + err = CreateScheduleTask(t.Context(), []*ActionSchedule{schedule}) + require.NoError(t, err) + + schedules, err := db.Find[ActionSchedule](t.Context(), FindScheduleOptions{OwnerID: user2.ID, RepoID: repo62.ID}) + require.NoError(t, err) + require.Len(t, schedules, 1) + + assert.NotZero(t, schedules[0].ID) + assert.Equal(t, ".forgejo/workflows/test.yaml", schedules[0].Title) + assert.Equal(t, "test.yaml", schedules[0].WorkflowID) + assert.Equal(t, ".forgejo/workflows", schedules[0].WorkflowDirectory) + assert.Equal(t, int64(-2), schedules[0].TriggerUserID) + assert.Equal(t, "main", schedules[0].Ref) + assert.Equal(t, "6af834a5bc97c1a337eb3a21d26903c5cdceca0c", schedules[0].CommitSHA) + assert.Equal(t, webhook.HookEventPush, schedules[0].Event) + assert.JSONEq(t, "{\"action\":\"schedule\"}", schedules[0].EventPayload) + assert.Equal(t, []byte(content), schedules[0].Content) + + specs, total, err := FindSpecs(t.Context(), FindSpecOptions{RepoID: repo62.ID}) + require.NoError(t, err) + + assert.Equal(t, int64(2), total) + + assert.NotZero(t, specs[0].ID) + assert.Equal(t, schedules[0].ID, specs[0].ScheduleID) + assert.Equal(t, timeutil.TimeStamp(1774699380), specs[0].Next) + assert.Equal(t, "3 13 * * *", specs[0].Spec) + assert.Equal(t, optional.Some("Europe/Paris"), specs[0].TimeZone) + assert.Zero(t, specs[0].Prev) + + assert.NotZero(t, specs[1].ID) + assert.Equal(t, schedules[0].ID, specs[1].ScheduleID) + assert.Equal(t, timeutil.TimeStamp(1774702920), specs[1].Next) + assert.Equal(t, "2 13 * * *", specs[1].Spec) + assert.Equal(t, optional.None[string](), specs[1].TimeZone) + assert.Zero(t, specs[1].Prev) +} diff --git a/models/forgejo_migrations/v15c_add_schedule_spec_time_zones.go b/models/forgejo_migrations/v15c_add_schedule_spec_time_zones.go new file mode 100644 index 0000000000..d72d585725 --- /dev/null +++ b/models/forgejo_migrations/v15c_add_schedule_spec_time_zones.go @@ -0,0 +1,31 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "forgejo.org/modules/optional" + + "xorm.io/xorm" +) + +func init() { + registerMigration(&Migration{ + Description: "add time zone support to action_schedule_spec", + Upgrade: addActionScheduleSpecTimeZone, + }) +} + +func addActionScheduleSpecTimeZone(x *xorm.Engine) error { + type ActionScheduleSpec struct { + TimeZone optional.Option[string] + } + + _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(ActionScheduleSpec)) + if err != nil { + return err + } + + _, err = x.Exec("ALTER TABLE action_schedule DROP COLUMN `specs`") + return err +} diff --git a/services/actions/TestServiceActions_startTask/action_schedule.yml b/services/actions/TestServiceActions_startTask/action_schedule.yml index d0e7234475..8102e3f9e3 100644 --- a/services/actions/TestServiceActions_startTask/action_schedule.yml +++ b/services/actions/TestServiceActions_startTask/action_schedule.yml @@ -2,8 +2,6 @@ - id: 1 title: schedule_title1 - specs: - - '* * * * *' repo_id: 4 owner_id: 2 workflow_id: 'workflow1.yml' @@ -23,8 +21,6 @@ - id: 2 title: schedule_title2 - specs: - - '* * * * *' repo_id: 4 owner_id: 2 workflow_id: 'workflow2.yml' diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index c6af9b5b82..5e048a83ad 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -10,6 +10,7 @@ import ( "fmt" "slices" "strings" + "time" actions_model "forgejo.org/models/actions" "forgejo.org/models/db" @@ -24,6 +25,7 @@ import ( "forgejo.org/modules/gitrepo" "forgejo.org/modules/json" "forgejo.org/modules/log" + "forgejo.org/modules/optional" "forgejo.org/modules/setting" api "forgejo.org/modules/structs" "forgejo.org/modules/util" @@ -574,6 +576,16 @@ func handleSchedules( continue } + now := time.Now() + specs := make([]*actions_model.ActionScheduleSpec, 0, len(schedules)) + for _, schedule := range schedules { + scheduleSpec, err := actions_model.NewActionScheduleSpec(schedule.Cron, optional.FromNonDefault(schedule.TimeZone), now) + if err != nil { + return err + } + specs = append(specs, scheduleSpec) + } + title := workflow.Name if len(title) < 1 { title = dwf.GetWorkflowPath() @@ -590,7 +602,7 @@ func handleSchedules( CommitSHA: commit.ID.String(), Event: input.Event, EventPayload: string(p), - Specs: schedules, + Specs: specs, Content: dwf.Content, } crons = append(crons, run) diff --git a/services/actions/schedule_tasks_test.go b/services/actions/schedule_tasks_test.go index 9bf964fd90..57ee6b955a 100644 --- a/services/actions/schedule_tasks_test.go +++ b/services/actions/schedule_tasks_test.go @@ -6,12 +6,14 @@ package actions import ( "context" "testing" + "time" actions_model "forgejo.org/models/actions" "forgejo.org/models/db" repo_model "forgejo.org/models/repo" "forgejo.org/models/unit" "forgejo.org/models/unittest" + "forgejo.org/modules/optional" "forgejo.org/modules/test" "forgejo.org/modules/timeutil" webhook_module "forgejo.org/modules/webhook" @@ -29,6 +31,9 @@ func TestServiceActions_startTask(t *testing.T) { // Load fixtures that are corrupted and create one valid scheduled workflow repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + spec, err := actions_model.NewActionScheduleSpec("* * * * *", optional.None[string](), time.Now()) + require.NoError(t, err) + workflowID := "some.yml" schedules := []*actions_model.ActionSchedule{ { @@ -42,7 +47,7 @@ func TestServiceActions_startTask(t *testing.T) { CommitSHA: "fakeSHA", Event: webhook_module.HookEventSchedule, EventPayload: "fakepayload", - Specs: []string{"* * * * *"}, + Specs: []*actions_model.ActionScheduleSpec{spec}, Content: []byte( ` jobs: @@ -57,7 +62,7 @@ jobs: require.Equal(t, 2, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) require.NoError(t, actions_model.CreateScheduleTask(t.Context(), schedules)) require.Equal(t, 3, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) - _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") + _, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") require.NoError(t, err) // After running startTasks an ActionRun row is created for the valid scheduled workflow @@ -291,6 +296,9 @@ func TestServiceActions_DynamicMatrix(t *testing.T) { // Load fixtures that are corrupted and create one valid scheduled workflow repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + spec, err := actions_model.NewActionScheduleSpec("* * * * *", optional.None[string](), time.Now()) + require.NoError(t, err) + workflowID := "some.yml" schedules := []*actions_model.ActionSchedule{ { @@ -304,7 +312,7 @@ func TestServiceActions_DynamicMatrix(t *testing.T) { CommitSHA: "fakeSHA", Event: webhook_module.HookEventSchedule, EventPayload: "fakepayload", - Specs: []string{"* * * * *"}, + Specs: []*actions_model.ActionScheduleSpec{spec}, Content: []byte( ` jobs: @@ -322,7 +330,7 @@ jobs: require.Equal(t, 2, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) require.NoError(t, actions_model.CreateScheduleTask(t.Context(), schedules)) require.Equal(t, 3, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) - _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") + _, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") require.NoError(t, err) // After running startTasks an ActionRun row is created for the valid scheduled workflow @@ -354,6 +362,9 @@ func TestServiceActions_RunsOnNeeds(t *testing.T) { // Load fixtures that are corrupted and create one valid scheduled workflow repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + spec, err := actions_model.NewActionScheduleSpec("* * * * *", optional.None[string](), time.Now()) + require.NoError(t, err) + workflowID := "some.yml" schedules := []*actions_model.ActionSchedule{ { @@ -366,7 +377,7 @@ func TestServiceActions_RunsOnNeeds(t *testing.T) { CommitSHA: "fakeSHA", Event: webhook_module.HookEventSchedule, EventPayload: "fakepayload", - Specs: []string{"* * * * *"}, + Specs: []*actions_model.ActionScheduleSpec{spec}, Content: []byte( ` jobs: @@ -381,7 +392,7 @@ jobs: require.Equal(t, 2, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) require.NoError(t, actions_model.CreateScheduleTask(t.Context(), schedules)) require.Equal(t, 3, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) - _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") + _, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") require.NoError(t, err) // After running startTasks an ActionRun row is created for the valid scheduled workflow @@ -440,6 +451,9 @@ func TestServiceActions_ExpandReusableWorkflow(t *testing.T) { // Load fixtures that are corrupted and create one valid scheduled workflow repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + spec, err := actions_model.NewActionScheduleSpec("* * * * *", optional.None[string](), time.Now()) + require.NoError(t, err) + workflowID := "some.yml" schedules := []*actions_model.ActionSchedule{ { @@ -452,7 +466,7 @@ func TestServiceActions_ExpandReusableWorkflow(t *testing.T) { CommitSHA: "fakeSHA", Event: webhook_module.HookEventSchedule, EventPayload: "fakepayload", - Specs: []string{"* * * * *"}, + Specs: []*actions_model.ActionScheduleSpec{spec}, Content: []byte( ` jobs: @@ -467,7 +481,7 @@ jobs: require.Equal(t, 2, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) require.NoError(t, actions_model.CreateScheduleTask(t.Context(), schedules)) require.Equal(t, 3, unittest.GetCount(t, actions_model.ActionScheduleSpec{})) - _, err := db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") + _, err = db.GetEngine(db.DefaultContext).Exec("UPDATE `action_schedule_spec` SET next = 1") require.NoError(t, err) // After running startTasks an ActionRun row is created for the valid scheduled workflow diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 4de8d625ab..914acadf14 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/url" + "slices" "strings" "testing" "time" @@ -23,6 +24,7 @@ import ( actions_module "forgejo.org/modules/actions" "forgejo.org/modules/git" "forgejo.org/modules/gitrepo" + "forgejo.org/modules/optional" "forgejo.org/modules/setting" api "forgejo.org/modules/structs" "forgejo.org/modules/test" @@ -1136,13 +1138,18 @@ func TestActionsWorkflowDispatchConcurrencyGroup(t *testing.T) { } func TestActionsScheduledWorkflow(t *testing.T) { + type expectedSpec struct { + cron string + timeZone optional.Option[string] + } + testCases := []struct { name string workflowID string workflowDirectory string workflowContent string expectedWorkflowTitle string - expectedCronSpecs []string + expectedCronSpecs []expectedSpec }{ { name: "GitHub", @@ -1158,7 +1165,7 @@ jobs: - run: echo OK `, expectedWorkflowTitle: ".github/workflows/scheduled.yml", - expectedCronSpecs: []string{"30 5,17 * * *"}, + expectedCronSpecs: []expectedSpec{{cron: "30 5,17 * * *", timeZone: optional.None[string]()}}, }, { name: "Gitea", @@ -1175,7 +1182,28 @@ jobs: - run: echo OK `, expectedWorkflowTitle: "My scheduled workflow", - expectedCronSpecs: []string{"* * * * *"}, + expectedCronSpecs: []expectedSpec{{cron: "* * * * *", timeZone: optional.None[string]()}}, + }, + { + name: "Forgejo with time zone", + workflowID: "tz.yml", + workflowDirectory: ".forgejo/workflows", + workflowContent: ` +on: + schedule: + - cron: "44 10 * * *" + - cron: "25 19 * * *" + timezone: Europe/Madrid +jobs: + test: + steps: + - run: echo OK +`, + expectedWorkflowTitle: ".forgejo/workflows/tz.yml", + expectedCronSpecs: []expectedSpec{ + {cron: "44 10 * * *", timeZone: optional.None[string]()}, + {cron: "25 19 * * *", timeZone: optional.Some("Europe/Madrid")}, + }, }, } onApplicationRun(t, func(t *testing.T, u *url.URL) { @@ -1201,7 +1229,6 @@ jobs: require.Len(t, schedules, 1) assert.Equal(t, testCase.expectedWorkflowTitle, schedules[0].Title) - assert.Equal(t, testCase.expectedCronSpecs, schedules[0].Specs) assert.Equal(t, repo.ID, schedules[0].RepoID) assert.Equal(t, repo.OwnerID, schedules[0].OwnerID) assert.Equal(t, testCase.workflowID, schedules[0].WorkflowID) @@ -1210,6 +1237,20 @@ jobs: assert.Equal(t, sha, schedules[0].CommitSHA) assert.Equal(t, webhook_module.HookEventPush, schedules[0].Event) assert.Equal(t, []byte(testCase.workflowContent), schedules[0].Content) + + specs, total, err := actions_model.FindSpecs(t.Context(), actions_model.FindSpecOptions{RepoID: repo.ID}) + require.NoError(t, err) + + assert.Equal(t, int64(len(testCase.expectedCronSpecs)), total) + + // The query to return cron specs orders by `id DESC`. + slices.Reverse(testCase.expectedCronSpecs) + + for i, expected := range testCase.expectedCronSpecs { + assert.Equal(t, schedules[0].ID, specs[i].ScheduleID) + assert.Equal(t, expected.cron, specs[i].Spec) + assert.Equal(t, expected.timeZone, specs[i].TimeZone) + } }) } })