Skip to content

Commit fb85caa

Browse files
author
Franck Rupin
committed
Fix comments in unittests
- This commit addresses comments done in unittests - It makes more readable some testCase and fixes version guards for all ct-*-limits mocked cli
1 parent 7ca590b commit fb85caa

File tree

1 file changed

+72
-60
lines changed

1 file changed

+72
-60
lines changed

ovs/datapath_test.go

Lines changed: 72 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ import (
1919
"testing"
2020
)
2121

22+
const (
23+
validTest = iota
24+
handleError
25+
unknownCommand
26+
)
27+
2228
// MockOvsCLI is mocking the "ovs-dpctl (Open vSwitch) 2.10.7" version
2329
type MockOvsCLI struct {
2430
Version uint8
@@ -46,8 +52,14 @@ func (cli *MockOvsCLI) Exec(args ...string) ([]byte, error) {
4652
out := "default limit=0\nzone=2,limit=1000000,count=0\nzone=3,limit=1000000,count=0"
4753
return []byte(out), nil
4854
case "ct-set-limits":
55+
if cli.Version < 10 {
56+
return []byte{}, errors.New("ovs-dpctl: unknown command 'ct-get-limits'; use --help for help")
57+
}
4958
return []byte{}, nil
5059
case "ct-del-limits":
60+
if cli.Version < 10 {
61+
return []byte{}, errors.New("ovs-dpctl: unknown command 'ct-get-limits'; use --help for help")
62+
}
5163
return []byte{}, nil
5264
default:
5365
return []byte{}, nil
@@ -198,14 +210,14 @@ func TestGetCTLimits(t *testing.T) {
198210
desc: "Test invalid ovs-dpctl version ",
199211
dp: &DataPathService{
200212
CLI: &MockOvsCLI{
201-
Version: 9,
213+
Version: unknownCommand,
202214
},
203215
},
204216
dpName: "ovs-system",
205217
zones: []uint64{2, 3},
206218
want: nil,
207219
err: "ovs-dpctl: unknown command 'ct-get-limits'; use --help for help",
208-
testCase: 9,
220+
testCase: unknownCommand,
209221
},
210222
{
211223
desc: "Test valid ovs-dpctl version ",
@@ -218,7 +230,7 @@ func TestGetCTLimits(t *testing.T) {
218230
zones: []uint64{2, 3},
219231
want: out,
220232
err: "",
221-
testCase: 10,
233+
testCase: validTest,
222234
},
223235
{
224236
desc: "Test valid ovs-dpctl ct-get-limits",
@@ -231,18 +243,18 @@ func TestGetCTLimits(t *testing.T) {
231243
zones: []uint64{},
232244
want: out,
233245
err: "datapath name argument is mandatory",
234-
testCase: 11,
246+
testCase: handleError,
235247
},
236248
}
237249
for _, tt := range tests {
238250
t.Run(tt.desc, func(t *testing.T) {
239251
results, err := tt.dp.GetCTLimits(tt.dpName, tt.zones)
240252
switch tt.testCase {
241-
case 9:
253+
case unknownCommand:
242254
if err.Error() != tt.err {
243255
t.Errorf("getting an error while trying to get conntrack limits %q", err.Error())
244256
}
245-
case 10:
257+
case validTest:
246258
if tt.want.defaultLimit["default"] != results.defaultLimit["default"] {
247259
t.Errorf("mismatched values, want %q but got %q", tt.want.defaultLimit["default"], results.defaultLimit["default"])
248260
}
@@ -251,7 +263,7 @@ func TestGetCTLimits(t *testing.T) {
251263
t.Errorf("mismatched values, want %q but got %q", tt.want.zones[i]["zone"], z["zone"])
252264
}
253265
}
254-
case 11:
266+
case handleError:
255267
if err != nil && err.Error() != tt.err {
256268
t.Errorf(err.Error())
257269
}
@@ -310,7 +322,7 @@ func TestGetCTLimitsWithBinary(t *testing.T) {
310322
zones: []uint64{2, 3},
311323
want: out,
312324
err: "",
313-
testCase: 10,
325+
testCase: validTest,
314326
},
315327
{
316328
desc: "Test valid ovs-dpctl ct-get-limits system@ovs-system",
@@ -319,7 +331,7 @@ func TestGetCTLimitsWithBinary(t *testing.T) {
319331
zones: []uint64{},
320332
want: outTest2,
321333
err: "",
322-
testCase: 10,
334+
testCase: validTest,
323335
},
324336
{
325337
desc: "Test valid ovs-dpctl ct-get-limits",
@@ -328,14 +340,14 @@ func TestGetCTLimitsWithBinary(t *testing.T) {
328340
zones: []uint64{},
329341
want: out,
330342
err: "datapath name argument is mandatory",
331-
testCase: 11,
343+
testCase: handleError,
332344
},
333345
}
334346
for _, tt := range tests {
335347
t.Run(tt.desc, func(t *testing.T) {
336348
results, err := tt.dp.GetCTLimits(tt.dpName, tt.zones)
337349
switch tt.testCase {
338-
case 10:
350+
case validTest:
339351
if tt.want.defaultLimit["default"] != results.defaultLimit["default"] {
340352
t.Errorf("mismatched values, want %q but got %q", tt.want.defaultLimit["default"], results.defaultLimit["default"])
341353
}
@@ -344,7 +356,7 @@ func TestGetCTLimitsWithBinary(t *testing.T) {
344356
t.Errorf("mismatched values, want %q but got %q", tt.want.zones[i]["zone"], z["zone"])
345357
}
346358
}
347-
case 11:
359+
case handleError:
348360
if err != nil && err.Error() != tt.err {
349361
t.Errorf(err.Error())
350362
}
@@ -528,12 +540,12 @@ func TestSetCTLimitsWithBinary(t *testing.T) {
528540

529541
func TestDelCTLimits(t *testing.T) {
530542
var tests = []struct {
531-
desc string
532-
dp *DataPathService
533-
zones []uint64
534-
dpName string
535-
err string
536-
res int
543+
desc string
544+
dp *DataPathService
545+
zones []uint64
546+
dpName string
547+
err string
548+
testCase uint8
537549
}{
538550
{
539551
desc: "Test del limit with valid argument",
@@ -542,9 +554,9 @@ func TestDelCTLimits(t *testing.T) {
542554
Version: 10,
543555
},
544556
},
545-
zones: []uint64{4, 3},
546-
dpName: "system@ovs-system",
547-
res: 0,
557+
zones: []uint64{4, 3},
558+
dpName: "system@ovs-system",
559+
testCase: validTest,
548560
},
549561
{
550562
desc: "Test del limit with datapath missing",
@@ -553,30 +565,30 @@ func TestDelCTLimits(t *testing.T) {
553565
Version: 10,
554566
},
555567
},
556-
zones: []uint64{4, 3},
557-
dpName: "",
558-
err: "datapath name is missing",
559-
res: 1,
568+
zones: []uint64{4, 3},
569+
dpName: "",
570+
err: "datapath name is missing",
571+
testCase: handleError,
560572
},
561573
{
562-
desc: "Test del limit with empty paramaters",
563-
dp: NewDataPathService(),
564-
zones: []uint64{},
565-
dpName: "",
566-
err: "datapath name is missing",
567-
res: 1,
574+
desc: "Test del limit with empty paramaters",
575+
dp: NewDataPathService(),
576+
zones: []uint64{},
577+
dpName: "",
578+
err: "datapath name is missing",
579+
testCase: handleError,
568580
},
569581
}
570582
for _, tt := range tests {
571583
t.Run(tt.desc, func(t *testing.T) {
572584
res, err := tt.dp.DelCTLimits(tt.dpName, tt.zones)
573-
switch tt.res {
574-
case 0:
585+
switch tt.testCase {
586+
case validTest:
575587
if err != nil {
576588
t.Errorf("got %q and %q", res, err.Error())
577589
}
578590

579-
case 1:
591+
case handleError:
580592
if err.Error() != tt.err {
581593
t.Errorf("got %q and %q", res, err.Error())
582594
}
@@ -594,47 +606,47 @@ func TestDelCTLimitsWithBinary(t *testing.T) {
594606
}
595607

596608
var tests = []struct {
597-
desc string
598-
dp *DataPathService
599-
zones []uint64
600-
dpName string
601-
err string
602-
res int
609+
desc string
610+
dp *DataPathService
611+
zones []uint64
612+
dpName string
613+
err string
614+
testCase uint8
603615
}{
604616
{
605-
desc: "Test del limit with valid argument",
606-
dp: NewDataPathService(),
607-
zones: []uint64{4, 3},
608-
dpName: "system@ovs-system",
609-
res: 0,
617+
desc: "Test del limit with valid argument",
618+
dp: NewDataPathService(),
619+
zones: []uint64{4, 3},
620+
dpName: "system@ovs-system",
621+
testCase: validTest,
610622
},
611623
{
612-
desc: "Test del limit with datapath missing",
613-
dp: NewDataPathService(),
614-
zones: []uint64{4, 3},
615-
dpName: "",
616-
err: "datapath name is missing",
617-
res: 1,
624+
desc: "Test del limit with datapath missing",
625+
dp: NewDataPathService(),
626+
zones: []uint64{4, 3},
627+
dpName: "",
628+
err: "datapath name is missing",
629+
testCase: handleError,
618630
},
619631
{
620-
desc: "Test del limit with empty paramaters",
621-
dp: NewDataPathService(),
622-
zones: []uint64{},
623-
dpName: "",
624-
err: "datapath name is missing",
625-
res: 1,
632+
desc: "Test del limit with empty paramaters",
633+
dp: NewDataPathService(),
634+
zones: []uint64{},
635+
dpName: "",
636+
err: "datapath name is missing",
637+
testCase: handleError,
626638
},
627639
}
628640
for _, tt := range tests {
629641
t.Run(tt.desc, func(t *testing.T) {
630642
res, err := tt.dp.DelCTLimits(tt.dpName, tt.zones)
631-
switch tt.res {
632-
case 0:
643+
switch tt.testCase {
644+
case validTest:
633645
if err != nil {
634646
t.Errorf("got %q and %q", res, err.Error())
635647
}
636648

637-
case 1:
649+
case handleError:
638650
if err.Error() != tt.err {
639651
t.Errorf("got %q and %q", res, err.Error())
640652
}

0 commit comments

Comments
 (0)