Skip to content

Commit

Permalink
Replace unit test assertions with Gomega matchers (#1046)
Browse files Browse the repository at this point in the history
Problem: There were still some existing unit tests that weren't using the Gomega
matcher library for assertions.

Solution: Changed existing tests to use the Gomega matcher library, replaced deprecated
NewGomegaWithT with NewWithT, and added sub-tests to existing tests.
  • Loading branch information
bjee19 authored Sep 11, 2023
1 parent dae5fac commit a39755d
Show file tree
Hide file tree
Showing 40 changed files with 231 additions and 306 deletions.
2 changes: 1 addition & 1 deletion cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type flagTestCase struct {
}

func testFlag(t *testing.T, cmd *cobra.Command, test flagTestCase) {
g := NewGomegaWithT(t)
g := NewWithT(t)
// discard any output generated by cobra
cmd.SetOut(io.Discard)
cmd.SetErr(io.Discard)
Expand Down
12 changes: 6 additions & 6 deletions cmd/gateway/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestValidateGatewayControllerName(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

err := validateGatewayControllerName(test.value)

Expand Down Expand Up @@ -115,7 +115,7 @@ func TestValidateResourceName(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

err := validateResourceName(test.value)

Expand Down Expand Up @@ -178,7 +178,7 @@ func TestValidateNamespaceName(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

err := validateNamespaceName(test.value)

Expand Down Expand Up @@ -240,7 +240,7 @@ func TestParseNamespacedResourceName(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

nsName, err := parseNamespacedResourceName(test.value)

Expand Down Expand Up @@ -283,7 +283,7 @@ func TestValidateIP(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

err := validateIP(tc.ip)
if !tc.expErr {
Expand Down Expand Up @@ -320,7 +320,7 @@ func TestValidatePort(t *testing.T) {

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

err := validatePort(tc.port)
if !tc.expErr {
Expand Down
2 changes: 1 addition & 1 deletion conformance/tests/conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
)

func TestConformance(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)
cfg, err := config.GetConfig()
g.Expect(err).To(BeNil())

Expand Down
8 changes: 3 additions & 5 deletions internal/framework/controller/filter/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ import (
func TestCreateSingleResourceFilter(t *testing.T) {
targetNsName := types.NamespacedName{Namespace: "test", Name: "resource"}

g := NewWithT(t)
filter := CreateSingleResourceFilter(targetNsName)
if filter == nil {
t.Fatal("TestCreateSingleResourceFilter() returned nil filter")
}
g.Expect(filter).ToNot(BeNil())

const expectedMsg = "Resource is ignored because this controller only supports a single resource " +
"test/resource of that type"
Expand Down Expand Up @@ -52,8 +51,7 @@ func TestCreateSingleResourceFilter(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)

g := NewWithT(t)
shouldProcess, msg := filter(test.nsname)
g.Expect(shouldProcess).To(Equal(test.expectedShouldProcess))
g.Expect(msg).To(Equal(test.expectedMsg))
Expand Down
12 changes: 5 additions & 7 deletions internal/framework/controller/index/endpointslice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package index
import (
"testing"

"github.com/google/go-cmp/cmp"
. "github.com/onsi/gomega"
v1 "k8s.io/api/core/v1"
discoveryV1 "k8s.io/api/discovery/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -42,18 +42,16 @@ func TestServiceNameIndexFunc(t *testing.T) {
}

for _, tc := range testcases {
g := NewWithT(t)
output := ServiceNameIndexFunc(tc.obj)
if diff := cmp.Diff(tc.expOutput, output); diff != "" {
t.Errorf("ServiceNameIndexFunc() mismatch on %q (-want +got):\n%s", tc.msg, diff)
}
g.Expect(output).To(Equal(tc.expOutput))
}
}

func TestServiceNameIndexFuncPanics(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Errorf("ServiceNameIndexFunc() did not panic")
}
g := NewWithT(t)
g.Expect(recover()).ShouldNot(BeNil())
}()

ServiceNameIndexFunc(&v1.Namespace{})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
)

func TestGatewayClassPredicate(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

p := GatewayClassPredicate{ControllerName: "nginx-ctlr"}

Expand Down
4 changes: 2 additions & 2 deletions internal/framework/controller/predicate/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) {

for _, tc := range testcases {
t.Run(tc.msg, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)
update := p.Update(event.UpdateEvent{
ObjectOld: tc.objectOld,
ObjectNew: tc.objectNew,
Expand All @@ -238,7 +238,7 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) {
}

func TestServicePortsChangedPredicate(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

p := ServicePortsChangedPredicate{}

Expand Down
2 changes: 1 addition & 1 deletion internal/framework/controller/register_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestRegister(t *testing.T) {

for _, test := range tests {
t.Run(test.msg, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

newReconciler := func(c controller.ReconcilerConfig) *controller.Reconciler {
g.Expect(c.Getter).To(BeIdenticalTo(test.fakes.mgr.GetClient()))
Expand Down
22 changes: 6 additions & 16 deletions internal/framework/events/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package events
import (
"testing"

"github.com/google/go-cmp/cmp"
. "github.com/onsi/gomega"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
)

func TestEventLoop_SwapBatches(t *testing.T) {
g := NewWithT(t)
eventLoop := NewEventLoop(nil, zap.New(), nil, nil)

eventLoop.currentBatch = EventBatch{
Expand All @@ -27,19 +28,8 @@ func TestEventLoop_SwapBatches(t *testing.T) {

eventLoop.swapBatches()

if l := len(eventLoop.currentBatch); l != 4 {
t.Errorf("EventLoop.swapBatches() mismatch. Expected 4 events in the current batch, got %d", l)
}

if diff := cmp.Diff(eventLoop.currentBatch, nextBatch); diff != "" {
t.Errorf("EventLoop.swapBatches() mismatch on current batch events (-want +got):\n%s", diff)
}

if l := len(eventLoop.nextBatch); l != 0 {
t.Errorf("EventLoop.swapBatches() mismatch. Expected 0 events in the next batch, got %d", l)
}

if c := cap(eventLoop.nextBatch); c != 3 {
t.Errorf("EventLoop.swapBatches() mismatch. Expected capacity of 3 in the next batch, got %d", c)
}
g.Expect(len(eventLoop.currentBatch)).To(Equal(len(nextBatch)))
g.Expect(eventLoop.currentBatch).To(Equal(nextBatch))
g.Expect(len(eventLoop.nextBatch)).To(Equal(0))
g.Expect(cap(eventLoop.nextBatch)).To(Equal(3))
}
2 changes: 1 addition & 1 deletion internal/framework/status/conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func CreateExpectedAPIConditions(
}

func TestConvertRouteConditions(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

var generation int64 = 1
transitionTime := metav1.NewTime(time.Now())
Expand Down
2 changes: 1 addition & 1 deletion internal/framework/status/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestPrepareGatewayStatus(t *testing.T) {
Addresses: []v1beta1.GatewayStatusAddress{podIP},
}

g := NewGomegaWithT(t)
g := NewWithT(t)

result := prepareGatewayStatus(status, "1.2.3.4", transitionTime)
g.Expect(helpers.Diff(expected, result)).To(BeEmpty())
Expand Down
2 changes: 1 addition & 1 deletion internal/framework/status/gatewayclass_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestPrepareGatewayClassStatus(t *testing.T) {
Conditions: CreateExpectedAPIConditions("Test", 1, transitionTime),
}

g := NewGomegaWithT(t)
g := NewWithT(t)

result := prepareGatewayClassStatus(status, transitionTime)
g.Expect(helpers.Diff(expected, result)).To(BeEmpty())
Expand Down
2 changes: 1 addition & 1 deletion internal/framework/status/httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestPrepareHTTPRouteStatus(t *testing.T) {
},
}

g := NewGomegaWithT(t)
g := NewWithT(t)

result := prepareHTTPRouteStatus(status, gatewayCtlrName, transitionTime)
g.Expect(helpers.Diff(expected, result)).To(BeEmpty())
Expand Down
8 changes: 4 additions & 4 deletions internal/mode/static/build_statuses_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestBuildStatuses(t *testing.T) {
},
}

g := NewGomegaWithT(t)
g := NewWithT(t)

var nginxReloadRes nginxReloadResult
result := buildStatuses(graph, nginxReloadRes)
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestBuildStatusesNginxErr(t *testing.T) {
},
}

g := NewGomegaWithT(t)
g := NewWithT(t)

nginxReloadRes := nginxReloadResult{error: errors.New("test error")}
result := buildStatuses(graph, nginxReloadRes)
Expand Down Expand Up @@ -349,7 +349,7 @@ func TestBuildGatewayClassStatuses(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

result := buildGatewayClassStatuses(test.gc, test.ignoredClasses)
g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty())
Expand Down Expand Up @@ -556,7 +556,7 @@ func TestBuildGatewayStatuses(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

result := buildGatewayStatuses(test.gateway, test.ignoredGateways, test.nginxReloadRes)
g.Expect(helpers.Diff(test.expected, result)).To(BeEmpty())
Expand Down
4 changes: 2 additions & 2 deletions internal/mode/static/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestPrepareFirstEventBatchPreparerArgs(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

objects, objectLists := prepareFirstEventBatchPreparerArgs(gcName, test.gwNsName)

Expand Down Expand Up @@ -112,7 +112,7 @@ func TestGetMetricsOptions(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)

metricsServerOptions := getMetricsOptions(test.metricsConfig)

Expand Down
17 changes: 7 additions & 10 deletions internal/mode/static/nginx/config/execute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,24 @@ package config
import (
"testing"

. "github.com/onsi/gomega"

"github.com/nginxinc/nginx-kubernetes-gateway/internal/mode/static/nginx/config/http"
)

func TestExecute(t *testing.T) {
g := NewWithT(t)
defer func() {
if r := recover(); r != nil {
t.Errorf("execute() panicked with %v", r)
}
g.Expect(recover()).Should(BeNil())
}()

bytes := execute(serversTemplate, []http.Server{})
if len(bytes) == 0 {
t.Error("template.execute() did not generate anything")
}
g.Expect(bytes).ToNot(BeEmpty())
}

func TestExecutePanics(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Error("template.execute() did not panic")
}
g := NewWithT(t)
g.Expect(recover()).ShouldNot(BeNil())
}()

_ = execute(serversTemplate, "not-correct-data")
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/nginx/config/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestGenerate(t *testing.T) {
},
},
}
g := NewGomegaWithT(t)
g := NewWithT(t)

generator := config.NewGeneratorImpl()

Expand Down
4 changes: 2 additions & 2 deletions internal/mode/static/nginx/config/maps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func TestExecuteMaps(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)
pathRules := []dataplane.PathRule{
{
MatchRules: []dataplane.MatchRule{
Expand Down Expand Up @@ -95,7 +95,7 @@ func TestExecuteMaps(t *testing.T) {
}

func TestBuildAddHeaderMaps(t *testing.T) {
g := NewGomegaWithT(t)
g := NewWithT(t)
pathRules := []dataplane.PathRule{
{
MatchRules: []dataplane.MatchRule{
Expand Down
Loading

0 comments on commit a39755d

Please sign in to comment.