Skip to content

Commit 02a9f6a

Browse files
chmouelclaude
andcommitted
chore: replace real time.Sleep with clockwork in unit tests
Inject clockwork.FakeClock into Gitea and GitHub provider retry/backoff loops so tests don't wait on real sleeps (~10s saved). Reduce LLM timeout test mock sleeps from 2s to 200ms. Fixes #2594 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent f8e4343 commit 02a9f6a

File tree

7 files changed

+51
-5
lines changed

7 files changed

+51
-5
lines changed

pkg/llm/providers/gemini/client_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ func TestAnalyzeTimeout(t *testing.T) {
463463

464464
client.httpClient = &http.Client{
465465
Transport: httptesting.RoundTripFunc(func(_ *http.Request) *http.Response {
466-
time.Sleep(2 * time.Second)
466+
time.Sleep(200 * time.Millisecond)
467467
return &http.Response{
468468
StatusCode: http.StatusOK,
469469
Body: io.NopCloser(strings.NewReader("")),

pkg/llm/providers/openai/client_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ func TestAnalyzeTimeout(t *testing.T) {
479479

480480
client.httpClient = &http.Client{
481481
Transport: httptesting.RoundTripFunc(func(_ *http.Request) *http.Response {
482-
time.Sleep(2 * time.Second)
482+
time.Sleep(200 * time.Millisecond)
483483
return &http.Response{
484484
StatusCode: http.StatusOK,
485485
Body: io.NopCloser(strings.NewReader("")),

pkg/provider/gitea/gitea.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"time"
1818

1919
"codeberg.org/mvdkleijn/forgejo-sdk/forgejo/v3"
20+
"github.com/jonboulle/clockwork"
2021
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
2122
"github.com/openshift-pipelines/pipelines-as-code/pkg/changedfiles"
2223
"github.com/openshift-pipelines/pipelines-as-code/pkg/events"
@@ -68,6 +69,7 @@ type Provider struct {
6869
triggerEvent string
6970
pacUserID int64 // user login used by PAC
7071
cachedChangedFiles *changedfiles.ChangedFiles
72+
clock clockwork.Clock
7173
}
7274

7375
func (v *Provider) Client() *forgejo.Client {
@@ -82,6 +84,13 @@ func (v *Provider) Client() *forgejo.Client {
8284
return v.giteaClient
8385
}
8486

87+
func (v *Provider) getClock() clockwork.Clock {
88+
if v.clock == nil {
89+
return clockwork.NewRealClock()
90+
}
91+
return v.clock
92+
}
93+
8594
func (v *Provider) SetGiteaClient(client *forgejo.Client) {
8695
v.giteaClient = client
8796
}
@@ -300,7 +309,7 @@ func (v *Provider) createStatusCommit(ctx context.Context, event *info.Event, pa
300309
// Only retry on transient "user does not exist" errors
301310
if strings.Contains(err.Error(), "user does not exist") {
302311
v.Logger.Warnf("CreateStatus failed with transient error, retrying %d/%d: %v", i+1, maxRetries, err)
303-
time.Sleep(time.Duration(i+1) * 500 * time.Millisecond)
312+
v.getClock().Sleep(time.Duration(i+1) * 500 * time.Millisecond)
304313
continue
305314
}
306315
return err

pkg/provider/gitea/gitea_test.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"time"
1616

1717
"github.com/google/go-cmp/cmp"
18+
"github.com/jonboulle/clockwork"
1819
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
1920
"github.com/openshift-pipelines/pipelines-as-code/pkg/changedfiles"
2021
"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
@@ -679,9 +680,11 @@ func TestProviderCreateStatusCommitRetryOnTransientError(t *testing.T) {
679680
_, _ = rw.Write([]byte(`{"state":"success"}`))
680681
})
681682

683+
fc := clockwork.NewFakeClock()
682684
v := &Provider{
683685
giteaClient: fakeclient,
684686
Logger: logger,
687+
clock: fc,
685688
}
686689

687690
event := &info.Event{
@@ -696,7 +699,18 @@ func TestProviderCreateStatusCommitRetryOnTransientError(t *testing.T) {
696699
Conclusion: "success",
697700
}
698701

699-
err := v.createStatusCommit(context.Background(), event, pacopts, status)
702+
ctx := context.Background()
703+
if strings.Contains(tt.errorMessage, "user does not exist") && tt.failCount > 0 {
704+
// Drive the fake clock only for retryable cases that actually sleep.
705+
go func() {
706+
for i := range 3 {
707+
fc.BlockUntilContext(ctx, 1) //nolint:errcheck
708+
fc.Advance(time.Duration(i+1) * 500 * time.Millisecond)
709+
}
710+
}()
711+
}
712+
713+
err := v.createStatusCommit(ctx, event, pacopts, status)
700714

701715
if tt.wantErr {
702716
assert.Assert(t, err != nil, "expected an error but got none")

pkg/provider/github/github.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ type Provider struct {
6464
cachedChangedFiles *changedfiles.ChangedFiles
6565
commitInfo *github.Commit
6666
pacUserLogin string // user/bot login used by PAC
67+
clock clockwork.Clock
6768
}
6869

6970
type skippedRun struct {
@@ -78,9 +79,17 @@ func New() *Provider {
7879
skippedRun: skippedRun{
7980
mutex: &sync.Mutex{},
8081
},
82+
clock: clockwork.NewRealClock(),
8183
}
8284
}
8385

86+
func (v *Provider) getClock() clockwork.Clock {
87+
if v.clock == nil {
88+
return clockwork.NewRealClock()
89+
}
90+
return v.clock
91+
}
92+
8493
func (v *Provider) Client() *github.Client {
8594
return v.ghClient
8695
}

pkg/provider/github/parse_payload.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ func (v *Provider) getPullRequestsWithCommit(ctx context.Context, sha, org, repo
269269
select {
270270
case <-ctx.Done():
271271
return nil, ctx.Err()
272-
case <-time.After(backoff):
272+
case <-v.getClock().After(backoff):
273273
}
274274
}
275275
}

pkg/provider/github/parse_payload_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ import (
66
"fmt"
77
"net/http"
88
"testing"
9+
"time"
910

1011
"github.com/google/go-github/v84/github"
12+
"github.com/jonboulle/clockwork"
1113
"gotest.tools/v3/assert"
1214
"gotest.tools/v3/env"
1315
corev1 "k8s.io/api/core/v1"
@@ -268,6 +270,18 @@ func TestGetPullRequestsWithCommit(t *testing.T) {
268270
}
269271
}
270272

273+
// Inject a fake clock for merge commit tests to avoid real backoff delays
274+
if tt.isMergeCommit {
275+
fc := clockwork.NewFakeClock()
276+
provider.clock = fc
277+
go func() {
278+
for i := range maxRetriesForMergeCommit {
279+
fc.BlockUntilContext(ctx, 1) //nolint:errcheck
280+
fc.Advance(time.Duration(1<<uint(i)) * time.Second)
281+
}
282+
}()
283+
}
284+
271285
prs, err := provider.getPullRequestsWithCommit(ctx, tt.sha, tt.org, tt.repo, tt.isMergeCommit)
272286
assert.Equal(t, err != nil, tt.wantErr)
273287
assert.Equal(t, len(prs), tt.wantPRsCount)

0 commit comments

Comments
 (0)