Skip to content

Commit ed4ae77

Browse files
f5yacobuccif5yacobucci
committed
Code review notes
Table testing Stderr Squash
1 parent 4f02f52 commit ed4ae77

File tree

2 files changed

+90
-85
lines changed

2 files changed

+90
-85
lines changed

‎cmd/gateway/setup.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func GatewayControllerParam(domain string, namespace string) ValidatorContext {
3838
fields := strings.Split(param, "/")
3939
l := len(fields)
4040
if l != 3 {
41-
return fmt.Errorf("unsupported path length, must be form DOMAIN/NAMESPACE/NAME")
41+
return errors.New("unsupported path length, must be form DOMAIN/NAMESPACE/NAME")
4242
}
4343

4444
for i := len(fields); i > 0; i-- {
@@ -55,7 +55,7 @@ func GatewayControllerParam(domain string, namespace string) ValidatorContext {
5555
fields = fields[1:]
5656
case 1:
5757
if fields[0] == "" {
58-
return fmt.Errorf("must provide a name")
58+
return errors.New("must provide a name")
5959
}
6060
}
6161
}
@@ -83,11 +83,11 @@ func MustValidateArguments(flagset *flag.FlagSet, validators ...ValidatorContext
8383
msgs := ValidateArguments(flagset, validators...)
8484
if msgs != nil {
8585
for i := range msgs {
86-
fmt.Printf("%s", msgs[i])
86+
fmt.Fprintf(os.Stderr, "%s", msgs[i])
8787
}
88-
fmt.Println("")
88+
fmt.Fprintln(os.Stderr, "")
8989

90-
fmt.Printf("Usage of %s:\n", os.Args[0])
90+
fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0])
9191
flag.PrintDefaults()
9292

9393
os.Exit(1)

‎cmd/gateway/setup_test.go

Lines changed: 85 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,35 @@ var _ = Describe("Main", func() {
121121
}) // Generic Validator
122122

123123
Describe("CLI argument validation", func() {
124+
type testCase struct {
125+
Param string
126+
Domain string
127+
ExpError bool
128+
}
129+
124130
var mockFlags *flag.FlagSet
125131
var gatewayCtlrName string
132+
133+
tester := func(t testCase) {
134+
err := mockFlags.Set(gatewayCtlrName, t.Param)
135+
Expect(err).ToNot(HaveOccurred())
136+
137+
v := GatewayControllerParam(domain, t.Domain)
138+
Expect(v.V).ToNot(BeNil())
139+
140+
err = v.V(mockFlags)
141+
if t.ExpError {
142+
Expect(err).To(HaveOccurred())
143+
} else {
144+
Expect(err).ToNot(HaveOccurred())
145+
}
146+
}
147+
runner := func(table []testCase) {
148+
for i := range table {
149+
tester(table[i])
150+
}
151+
}
152+
126153
BeforeEach(func() {
127154
domain = "k8s-gateway.nginx.org"
128155
gatewayCtlrName = "gateway-ctlr-name"
@@ -136,97 +163,75 @@ var _ = Describe("Main", func() {
136163
mockFlags = nil
137164
})
138165
It("should parse full gateway-ctlr-name", func() {
139-
err := mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/nginx-gateway/my-gateway")
140-
Expect(err).ToNot(HaveOccurred())
141-
142-
v := GatewayControllerParam(domain, "nginx-gateway")
143-
Expect(v.V).ToNot(BeNil())
144-
145-
err = v.V(mockFlags)
146-
Expect(err).ToNot(HaveOccurred())
166+
t := testCase{
167+
"k8s-gateway.nginx.org/nginx-gateway/my-gateway",
168+
"nginx-gateway",
169+
false,
170+
}
171+
tester(t)
147172
}) // should parse full gateway-ctlr-name
148173

149174
It("should fail with too many path elements", func() {
150-
err := mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/nginx-gateway/my-gateway/broken")
151-
Expect(err).ToNot(HaveOccurred())
152-
153-
v := GatewayControllerParam(domain, "nginx-gateway")
154-
Expect(v.V).ToNot(BeNil())
155-
156-
err = v.V(mockFlags)
157-
Expect(err).To(HaveOccurred())
175+
t := testCase{
176+
"k8s-gateway.nginx.org/nginx-gateway/my-gateway/broken",
177+
"nginx-gateway",
178+
true,
179+
}
180+
tester(t)
158181
}) // should fail with too many path elements
159182

160183
It("should fail with too few path elements", func() {
161-
err := mockFlags.Set(gatewayCtlrName, "nginx-gateway/my-gateway")
162-
Expect(err).ToNot(HaveOccurred())
163-
164-
v := GatewayControllerParam(domain, "nginx-gateway")
165-
Expect(v.V).ToNot(BeNil())
166-
167-
err = v.V(mockFlags)
168-
Expect(err).To(HaveOccurred())
169-
170-
err = mockFlags.Set(gatewayCtlrName, "my-gateway")
171-
Expect(err).ToNot(HaveOccurred())
172-
173-
v = GatewayControllerParam(domain, "nginx-gateway")
174-
Expect(v.V).ToNot(BeNil())
184+
table := []testCase{
185+
{
186+
Param: "nginx-gateway/my-gateway",
187+
Domain: "nginx-gateway",
188+
ExpError: true,
189+
},
190+
{
191+
Param: "my-gateway",
192+
Domain: "nginx-gateway",
193+
ExpError: true,
194+
},
195+
}
175196

176-
err = v.V(mockFlags)
177-
Expect(err).To(HaveOccurred())
197+
runner(table)
178198
}) // should fail with too few path elements
179199

180200
It("should verify constraints", func() {
181-
// bad domain
182-
err := mockFlags.Set(gatewayCtlrName, "invalid-domain/nginx-gateway/my-gateway")
183-
Expect(err).ToNot(HaveOccurred())
184-
185-
v := GatewayControllerParam(domain, "nginx-gateway")
186-
Expect(v.V).ToNot(BeNil())
187-
188-
err = v.V(mockFlags)
189-
Expect(err).To(HaveOccurred())
190-
191-
// bad domain
192-
err = mockFlags.Set(gatewayCtlrName, "/default/my-gateway")
193-
Expect(err).ToNot(HaveOccurred())
194-
195-
v = GatewayControllerParam(domain, "nginx-gateway")
196-
Expect(v.V).ToNot(BeNil())
197-
198-
err = v.V(mockFlags)
199-
Expect(err).To(HaveOccurred())
200-
201-
// bad namespace
202-
err = mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/default/my-gateway")
203-
Expect(err).ToNot(HaveOccurred())
204-
205-
v = GatewayControllerParam(domain, "nginx-gateway")
206-
Expect(v.V).ToNot(BeNil())
207-
208-
err = v.V(mockFlags)
209-
Expect(err).To(HaveOccurred())
210-
211-
// bad namespace
212-
err = mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org//my-gateway")
213-
Expect(err).ToNot(HaveOccurred())
214-
215-
v = GatewayControllerParam(domain, "nginx-gateway")
216-
Expect(v.V).ToNot(BeNil())
217-
218-
err = v.V(mockFlags)
219-
Expect(err).To(HaveOccurred())
220-
221-
// bad name
222-
err = mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/default/")
223-
Expect(err).ToNot(HaveOccurred())
224-
225-
v = GatewayControllerParam(domain, "nginx-gateway")
226-
Expect(v.V).ToNot(BeNil())
201+
table := []testCase{
202+
{
203+
// bad domain
204+
Param: "invalid-domain/nginx-gateway/my-gateway",
205+
Domain: "nginx-gateway",
206+
ExpError: true,
207+
},
208+
{
209+
// bad domain
210+
Param: "/default/my-gateway",
211+
Domain: "nginx-gateway",
212+
ExpError: true,
213+
},
214+
{
215+
// bad namespace
216+
Param: "k8s-gateway.nginx.org/default/my-gateway",
217+
Domain: "nginx-gateway",
218+
ExpError: true,
219+
},
220+
{
221+
// bad namespace
222+
Param: "k8s-gateway.nginx.org//my-gateway",
223+
Domain: "nginx-gateway",
224+
ExpError: true,
225+
},
226+
{
227+
// bad name
228+
Param: "k8s-gateway.nginx.org/default/",
229+
Domain: "nginx-gateway",
230+
ExpError: true,
231+
},
232+
}
227233

228-
err = v.V(mockFlags)
229-
Expect(err).To(HaveOccurred())
234+
runner(table)
230235
}) // should verify constraints
231236
}) // CLI argument validation
232237
}) // end Main

0 commit comments

Comments
 (0)