Skip to content

Commit

Permalink
refactor: remove unnecessary defers in tests (#4713)
Browse files Browse the repository at this point in the history
Since Go 1.14 gomock will use `T.Cleanup` automatically.

This is the results of running:
```sh
find . -type f -name "*.go" -exec sed -i '' '/defer ctrl\.Finish()/d' {} +
find . -type f -name "*.go" -exec sed -i '' '/defer ctlr\.Finish()/d' {} +
find . -type f -name "*.go" -exec sed -i '' '/defer mctrl\.Finish()/d' {} +
find . -type f -name "*.go" -exec sed -i '' '/defer rootCtrl\.Finish()/d' {} +
```
And cleaning up unused variables by hand.
  • Loading branch information
romshark authored Feb 25, 2025
1 parent abd39af commit 90da24d
Show file tree
Hide file tree
Showing 86 changed files with 13 additions and 191 deletions.
1 change: 0 additions & 1 deletion control/beacon/beacon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ func TestBeaconDiversity(t *testing.T) {
},
}
mctrl := gomock.NewController(t)
defer mctrl.Finish()

g := graph.NewDefaultGraph(mctrl)
bseg := testBeacon(g, tests[0].beacon...)
Expand Down
4 changes: 0 additions & 4 deletions control/beacon/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ func testStoreSelection(t *testing.T,
methodToTest func(store *beacon.Store) ([]beacon.Beacon, error)) {

mctrl := gomock.NewController(t)
defer mctrl.Finish()
g := graph.NewDefaultGraph(mctrl)

// Ensure remote out if is set in last AS entry.
Expand Down Expand Up @@ -111,7 +110,6 @@ func testStoreSelection(t *testing.T,
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
db := mock_beacon.NewMockDB(mctrl)
policies := beacon.Policies{
Prop: beacon.Policy{BestSetSize: test.bestSize},
Expand Down Expand Up @@ -158,7 +156,6 @@ func testCoreStoreSelection(t *testing.T,
methodToTest func(store *beacon.CoreStore) ([]beacon.Beacon, error)) {

mctrl := gomock.NewController(t)
defer mctrl.Finish()
g := graph.NewDefaultGraph(mctrl)

// Ensure remote out if is set in last AS entry.
Expand Down Expand Up @@ -228,7 +225,6 @@ func testCoreStoreSelection(t *testing.T,
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
db := mock_beacon.NewMockDB(mctrl)
policies := beacon.CorePolicies{
Prop: beacon.Policy{BestSetSize: test.bestSize},
Expand Down
9 changes: 0 additions & 9 deletions control/beaconing/extender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"testing"
"time"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -91,8 +90,6 @@ func TestDefaultExtenderExtend(t *testing.T) {
}
for name, tc := range testsCases {
t.Run(name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
// Setup interfaces with active parent, child and one peer interface.
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
for _, peer := range tc.peers {
Expand Down Expand Up @@ -167,8 +164,6 @@ func TestDefaultExtenderExtend(t *testing.T) {
})
}
t.Run("the maximum expiration time is respected", func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
require.NoError(t, err)
ext := &beaconing.DefaultExtender{
Expand Down Expand Up @@ -261,8 +256,6 @@ func TestDefaultExtenderExtend(t *testing.T) {
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
require.NoError(t, err)
ext := &beaconing.DefaultExtender{
Expand Down Expand Up @@ -332,8 +325,6 @@ func TestDefaultExtenderExtend(t *testing.T) {
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
ext := &beaconing.DefaultExtender{
IA: topo.IA(),
Expand Down
2 changes: 0 additions & 2 deletions control/beaconing/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func TestHandlerHandleBeacon(t *testing.T) {

validBeacon := func() beacon.Beacon {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
g := graph.NewDefaultGraph(mctrl)
return beacon.Beacon{
Segment: testSegment(g, []uint16{graph.If_220_X_120_B, graph.If_120_A_110_X}),
Expand Down Expand Up @@ -257,7 +256,6 @@ func TestHandlerHandleBeacon(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()
mctrl := gomock.NewController(t)
defer mctrl.Finish()

handler := beaconing.Handler{
LocalIA: localIA,
Expand Down
2 changes: 0 additions & 2 deletions control/beaconing/originator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ func TestOriginatorRun(t *testing.T) {
}
t.Run("run originates ifID packets on all active interfaces", func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
senderFactory := mock_beaconing.NewMockSenderFactory(mctrl)
o := beaconing.Originator{
Expand Down Expand Up @@ -126,7 +125,6 @@ func TestOriginatorRun(t *testing.T) {
})
t.Run("Fast recovery", func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
senderFactory := mock_beaconing.NewMockSenderFactory(mctrl)
sender := mock_beaconing.NewMockSender(mctrl)
Expand Down
3 changes: 0 additions & 3 deletions control/beaconing/propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func TestPropagatorRunNonCore(t *testing.T) {
}

mctrl := gomock.NewController(t)
defer mctrl.Finish()
topo, err := topology.FromJSONFile(topoNonCore)
require.NoError(t, err)
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
Expand Down Expand Up @@ -126,7 +125,6 @@ func TestPropagatorRunCore(t *testing.T) {
}

mctrl := gomock.NewController(t)
defer mctrl.Finish()
topo, err := topology.FromJSONFile(topoCore)
require.NoError(t, err)
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
Expand Down Expand Up @@ -212,7 +210,6 @@ func TestPropagatorFastRecovery(t *testing.T) {
{graph.If_130_B_120_A, graph.If_120_A_110_X},
}
mctrl := gomock.NewController(t)
defer mctrl.Finish()
topo, err := topology.FromJSONFile(topoCore)
require.NoError(t, err)
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
Expand Down
3 changes: 0 additions & 3 deletions control/beaconing/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ func TestRegistrarRun(t *testing.T) {
for _, test := range testsLocal {
t.Run(test.name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()
topo, err := topology.FromJSONFile(test.fn)
require.NoError(t, err)
intfs := ifstate.NewInterfaces(interfaceInfos(topo), ifstate.Config{})
Expand Down Expand Up @@ -173,7 +172,6 @@ func TestRegistrarRun(t *testing.T) {
for _, test := range testsRemote {
t.Run(test.name, func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()

topo, err := topology.FromJSONFile(test.fn)
require.NoError(t, err)
Expand Down Expand Up @@ -275,7 +273,6 @@ func TestRegistrarRun(t *testing.T) {

t.Run("Faulty beacons are not sent", func(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()

topo, err := topology.FromJSONFile(topoNonCore)
require.NoError(t, err)
Expand Down
4 changes: 0 additions & 4 deletions control/drkey/grpc/drkey_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func TestDRKeySV(t *testing.T) {
sv, targetResp := getSVandResp(t)

ctrl := gomock.NewController(t)
defer ctrl.Finish()

serviceStore := mock_grpc.NewMockEngine(ctrl)
serviceStore.EXPECT().GetSecretValue(gomock.Any(), gomock.Any()).Return(sv, nil)
Expand Down Expand Up @@ -305,7 +304,6 @@ func TestLevel1(t *testing.T) {

func TestASHost(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := mock_grpc.NewMockEngine(ctrl)
engine.EXPECT().DeriveASHost(gomock.Any(), gomock.Any()).Return(
Expand Down Expand Up @@ -336,7 +334,6 @@ func TestASHost(t *testing.T) {

func TestHostAS(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := mock_grpc.NewMockEngine(ctrl)
engine.EXPECT().DeriveHostAS(gomock.Any(), gomock.Any()).Return(
Expand Down Expand Up @@ -367,7 +364,6 @@ func TestHostAS(t *testing.T) {

func TestHostHost(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := mock_grpc.NewMockEngine(ctrl)
engine.EXPECT().DeriveHostHost(gomock.Any(), gomock.Any()).Return(
Expand Down
1 change: 0 additions & 1 deletion control/drkey/grpc/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ func TestLevel1KeyFetching(t *testing.T) {
require.NoError(t, err)

ctrl := gomock.NewController(t)
defer ctrl.Finish()

lvl1db := mock_grpc.NewMockEngine(ctrl)
lvl1db.EXPECT().DeriveLevel1(gomock.Any()).AnyTimes().Return(drkey.Level1Key{}, nil)
Expand Down
1 change: 0 additions & 1 deletion control/drkey/prefetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ var _ periodic.Task = (*cs_drkey.Prefetcher)(nil)

func TestPrefetcherRun(t *testing.T) {
mctrl := gomock.NewController(t)
defer mctrl.Finish()

mock_engine := mock_drkey.NewMockLevel1Engine(mctrl)

Expand Down
2 changes: 0 additions & 2 deletions control/drkey/service_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ func TestDeriveHostAS(t *testing.T) {
defer lvl1db.Close()

mctrl := gomock.NewController(t)
defer mctrl.Finish()

fetcher := mock_drkey.NewMockFetcher(mctrl)
fetcher.EXPECT().Level1(gomock.Any(), gomock.Any()).DoAndReturn(
Expand Down Expand Up @@ -182,7 +181,6 @@ func TestGetLevel1Key(t *testing.T) {
copy(secondLevel1Key.Key[:], k)

mctrl := gomock.NewController(t)
defer mctrl.Finish()

fetcher := mock_drkey.NewMockFetcher(mctrl)
gomock.InOrder(
Expand Down
1 change: 0 additions & 1 deletion control/mgmtapi/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,6 @@ func TestAPI(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

req, err := http.NewRequest("GET", tc.RequestURL, nil)
require.NoError(t, err)
Expand Down
1 change: 0 additions & 1 deletion control/segreq/authoritative_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ func TestAuthoritativeClassify(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

p := AuthoritativeLookup{
LocalIA: test.LocalIA,
Expand Down
1 change: 0 additions & 1 deletion control/segreq/forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ func TestForwarderClassify(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

f := ForwardingLookup{LocalIA: test.LocalIA, CoreChecker: newMockCoreChecker(ctrl)}
segType, err := f.classify(context.Background(), test.Request.Src, test.Request.Dst)
Expand Down
1 change: 0 additions & 1 deletion control/segreq/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ func TestCoreChecker(t *testing.T) {
for name, test := range tests {
t.Run(name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
i := mock_trust.NewMockInspector(ctrl)
test.PrepareInspector(i)
c := segreq.CoreChecker{Inspector: i}
Expand Down
1 change: 0 additions & 1 deletion control/trust/crypto_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ func TestChainLoaderChains(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

dir, db := tc.prepare(t, ctrl)
loader := cstrust.CryptoLoader{
Expand Down
1 change: 0 additions & 1 deletion daemon/drkey/grpc/fetching_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var _ sd_drkey.Fetcher = (*sd_grpc.Fetcher)(nil)

func TestGetHostHost(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

now := time.Now().UTC()
epochBegin := timestamppb.New(now)
Expand Down
6 changes: 0 additions & 6 deletions gateway/control/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ func TestEngineRun(t *testing.T) {
t.Run("double run", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := &control.Engine{
SessionConfigs: nil,
Expand All @@ -55,7 +54,6 @@ func TestEngineRun(t *testing.T) {
t.Run("nil routing table", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := &control.Engine{
PathMonitor: mock_control.NewMockPathMonitor(ctrl),
Expand All @@ -69,7 +67,6 @@ func TestEngineRun(t *testing.T) {
t.Run("nil path monitor", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := &control.Engine{
SessionConfigs: nil,
Expand All @@ -84,7 +81,6 @@ func TestEngineRun(t *testing.T) {
t.Run("nil probe conn factory", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := &control.Engine{
SessionConfigs: nil,
Expand All @@ -99,7 +95,6 @@ func TestEngineRun(t *testing.T) {
t.Run("nil dataplane session factory", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := &control.Engine{
SessionConfigs: nil,
Expand All @@ -114,7 +109,6 @@ func TestEngineRun(t *testing.T) {
t.Run("close before run", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

engine := &control.Engine{
SessionConfigs: nil,
Expand Down
5 changes: 0 additions & 5 deletions gateway/control/enginecontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ func TestEngineControllerRun(t *testing.T) {
t.Run("double run", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

configurationUpdates := make(chan []*control.SessionConfig)
routingTableSwapper := mock_control.NewMockRoutingTableSwapper(ctrl)
Expand Down Expand Up @@ -63,7 +62,6 @@ func TestEngineControllerRun(t *testing.T) {
t.Run("nil configuration updates", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

routingTableSwapper := mock_control.NewMockRoutingTableSwapper(ctrl)
routingTableFactory := mock_control.NewMockRoutingTableFactory(ctrl)
Expand All @@ -82,7 +80,6 @@ func TestEngineControllerRun(t *testing.T) {
t.Run("nil routing table swapper", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

configurationUpdates := make(chan []*control.SessionConfig)
routingTableFactory := mock_control.NewMockRoutingTableFactory(ctrl)
Expand All @@ -101,7 +98,6 @@ func TestEngineControllerRun(t *testing.T) {
t.Run("nil routing table factory", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

configurationUpdates := make(chan []*control.SessionConfig)
routingTableSwapper := mock_control.NewMockRoutingTableSwapper(ctrl)
Expand All @@ -120,7 +116,6 @@ func TestEngineControllerRun(t *testing.T) {
t.Run("nil engine factory", func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

configurationUpdates := make(chan []*control.SessionConfig)
routingTableSwapper := mock_control.NewMockRoutingTableSwapper(ctrl)
Expand Down
1 change: 0 additions & 1 deletion gateway/control/grpc/prefix_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ func TestIPPrefixServerPrefixes(t *testing.T) {
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
s := grpc.IPPrefixServer{
LocalIA: local,
Advertiser: tc.Advertiser(t, ctrl),
Expand Down
1 change: 0 additions & 1 deletion gateway/control/grpc/probeserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (

func TestControlDispatcher(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

src := &snet.UDPAddr{IA: addr.MustParseIA("1-ff00:0:110")}

Expand Down
1 change: 0 additions & 1 deletion gateway/control/prefixesfilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ accept 1-ff00:0:113 1-ff00:0:110 10.3.0.0/25`))
t.Run(name, func(t *testing.T) {
t.Parallel()
ctrl := gomock.NewController(t)
defer ctrl.Finish()

f := tc.CreateFilter(t, ctrl)
for _, input := range tc.Inputs {
Expand Down
Loading

0 comments on commit 90da24d

Please sign in to comment.