From 92f1b6fdd2851bb522b8b153d77f51eaf67ad309 Mon Sep 17 00:00:00 2001 From: Andreas Ahlenstorf Date: Tue, 7 Apr 2026 05:03:26 +0200 Subject: [PATCH] test: fix test that was supposed to test DST behaviour but did not (#12007) https://codeberg.org/forgejo/forgejo/pulls/11851 introduced tests that verify the scheduling of Forgejo Actions workflows during daylight saving time (DST) changes. Unfortunately, one test didn't test what it was supposed to because it used a reference time in UTC that was already after the clock change has happened. This change also adds tests that verify that `NewActionScheduleSpec()` respects time zones when calculating the initial execution time of a scheduled workflow. ## 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. - [ ] 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 - [ ] 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. - [x] 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/12007 Reviewed-by: Mathieu Fenniak Co-authored-by: Andreas Ahlenstorf Co-committed-by: Andreas Ahlenstorf --- models/actions/schedule_spec_test.go | 50 ++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/models/actions/schedule_spec_test.go b/models/actions/schedule_spec_test.go index eb3a83d0a6..6b9db9f39a 100644 --- a/models/actions/schedule_spec_test.go +++ b/models/actions/schedule_spec_test.go @@ -13,6 +13,44 @@ import ( "github.com/stretchr/testify/require" ) +func TestActionScheduleSpec_NewActionScheduleSpec(t *testing.T) { + tests := []struct { + name string + refTime time.Time + cronPattern string + timeZone string + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "without timezone", + refTime: time.Date(2026, 4, 6, 11, 56, 0, 0, time.UTC), + cronPattern: "58 14 * * *", + want: "2026-04-06T14:58:00Z", + wantErr: assert.NoError, + }, + { + name: "with separate timezone", + refTime: time.Date(2026, 4, 6, 11, 56, 0, 0, time.UTC), + cronPattern: "58 14 * * *", + timeZone: "Europe/Tallinn", // +03 (EEST) + want: "2026-04-06T11:58:00Z", + wantErr: assert.NoError, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s, err := NewActionScheduleSpec(test.cronPattern, optional.FromNonDefault(test.timeZone), test.refTime) + test.wantErr(t, err) + + if err == nil { + assert.Equal(t, test.want, s.Next.AsTime().UTC().Format(time.RFC3339)) + } + }) + } +} + func TestActionScheduleSpec_Parse(t *testing.T) { // Mock the local timezone is not UTC local := time.Local @@ -88,24 +126,24 @@ func TestActionScheduleSpec_Parse(t *testing.T) { // 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. + refTime: time.Date(2025, 3, 30, 0, 55, 0, 0, time.UTC), // 01:55 local time + 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. + refTime: time.Date(2025, 10, 26, 0, 5, 0, 0, time.UTC), // 02:05 local time + 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. + refTime: time.Date(2025, 10, 26, 1, 5, 0, 0, time.UTC), // 02:05 local time + 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,