Skip to content

Commit c304fa2

Browse files
committed
Change naming and add subtests
1 parent c62b364 commit c304fa2

File tree

5 files changed

+95
-116
lines changed

5 files changed

+95
-116
lines changed

internal/framework/status/conditionWithContextFunc_test.go

-94
This file was deleted.

internal/framework/status/status_updater.go renamed to internal/framework/status/k8s_updater.go

+3-8
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,11 @@ import (
66
"sigs.k8s.io/controller-runtime/pkg/client"
77
)
88

9-
//go:generate go run github.com./maxbrunsfeld/counterfeiter/v6 . StatusUpdater
9+
//go:generate go run github.com./maxbrunsfeld/counterfeiter/v6 . K8sUpdater
1010

11-
// StatusUpdater updates a resource from the k8s API.
11+
// K8sUpdater updates a resource from the k8s API.
1212
// It allows us to mock the client.Reader.Status.Update method.
13-
//
14-
// There already is an interface in updater.go that is named "Updater"
15-
// so naming this StatusUpdater works, but the linter does not like
16-
// the interface name starting with the package name.
17-
// nolint:revive
18-
type StatusUpdater interface {
13+
type K8sUpdater interface {
1914
// Update is from client.StatusClient.SubResourceWriter.
2015
Update(ctx context.Context, obj client.Object, opts ...client.SubResourceUpdateOption) error
2116
}

internal/framework/status/statusfakes/fake_status_updater.go renamed to internal/framework/status/statusfakes/fake_k8s_updater.go

+10-10
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/framework/status/updater.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ func (upd *UpdaterImpl) writeStatuses(
238238
Cap: time.Millisecond * 3000,
239239
},
240240
// Function returns true if the condition is satisfied, or an error if the loop should be aborted.
241-
ConditionWithContextFunc(upd.cfg.Client, upd.cfg.Client.Status(), nsname, obj, upd.cfg.Logger, statusSetter),
241+
NewRetryUpdateFunc(upd.cfg.Client, upd.cfg.Client.Status(), nsname, obj, upd.cfg.Logger, statusSetter),
242242
)
243243
if err != nil && !errors.Is(err, context.Canceled) {
244244
upd.cfg.Logger.Error(
@@ -250,7 +250,7 @@ func (upd *UpdaterImpl) writeStatuses(
250250
}
251251
}
252252

253-
// ConditionWithContextFunc returns a function which will be used in wait.ExponentialBackoffWithContext.
253+
// NewRetryUpdateFunc returns a function which will be used in wait.ExponentialBackoffWithContext.
254254
// The function will attempt to Update a kubernetes resource and will be retried in
255255
// wait.ExponentialBackoffWithContext if an error occurs. Exported for testing purposes.
256256
//
@@ -259,9 +259,9 @@ func (upd *UpdaterImpl) writeStatuses(
259259
// the linter will complain if we return nil if an error was found.
260260
//
261261
//nolint:nilerr
262-
func ConditionWithContextFunc(
262+
func NewRetryUpdateFunc(
263263
getter controller.Getter,
264-
updater StatusUpdater,
264+
updater K8sUpdater,
265265
nsname types.NamespacedName,
266266
obj client.Object,
267267
logger logr.Logger,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package status_test
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
. "github.com./onsi/gomega"
9+
apierrors "k8s.io/apimachinery/pkg/api/errors"
10+
"k8s.io/apimachinery/pkg/runtime/schema"
11+
"k8s.io/apimachinery/pkg/types"
12+
"sigs.k8s.io/controller-runtime/pkg/client"
13+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
14+
"sigs.k8s.io/gateway-api/apis/v1beta1"
15+
16+
"github.com./nginxinc/nginx-gateway-fabric/internal/framework/controller/controllerfakes"
17+
"github.com./nginxinc/nginx-gateway-fabric/internal/framework/status"
18+
"github.com./nginxinc/nginx-gateway-fabric/internal/framework/status/statusfakes"
19+
)
20+
21+
func TestNewRetryUpdateFunc(t *testing.T) {
22+
tests := []struct {
23+
getReturns error
24+
updateReturns error
25+
name string
26+
expConditionPassed bool
27+
}{
28+
{
29+
getReturns: errors.New("failed to get resource"),
30+
updateReturns: nil,
31+
name: "get fails",
32+
expConditionPassed: false,
33+
},
34+
{
35+
getReturns: apierrors.NewNotFound(schema.GroupResource{}, "not found"),
36+
updateReturns: nil,
37+
name: "get fails and apierrors is not found",
38+
expConditionPassed: true,
39+
},
40+
{
41+
getReturns: nil,
42+
updateReturns: errors.New("failed to update resource"),
43+
name: "update fails",
44+
expConditionPassed: false,
45+
},
46+
{
47+
getReturns: nil,
48+
updateReturns: nil,
49+
name: "nothing fails",
50+
expConditionPassed: true,
51+
},
52+
}
53+
for _, test := range tests {
54+
t.Run(test.name, func(t *testing.T) {
55+
g := NewWithT(t)
56+
fakeStatusUpdater := &statusfakes.FakeK8sUpdater{}
57+
fakeGetter := &controllerfakes.FakeGetter{}
58+
fakeStatusUpdater.UpdateReturns(test.updateReturns)
59+
fakeGetter.GetReturns(test.getReturns)
60+
f := status.NewRetryUpdateFunc(
61+
fakeGetter,
62+
fakeStatusUpdater,
63+
types.NamespacedName{},
64+
&v1beta1.GatewayClass{},
65+
zap.New(),
66+
func(client.Object) {})
67+
conditionPassed, err := f(context.Background())
68+
69+
// For now, the function should always return nil
70+
g.Expect(err).ToNot(HaveOccurred())
71+
if test.expConditionPassed {
72+
g.Expect(conditionPassed).To(BeTrue())
73+
} else {
74+
g.Expect(conditionPassed).To(BeFalse())
75+
}
76+
})
77+
}
78+
}

0 commit comments

Comments
 (0)