Skip to content

Commit

Permalink
Code review notes
Browse files Browse the repository at this point in the history 
Table testing
Stderr

Squash
  • Loading branch information
@f5yacobucci
f5yacobucci committed Nov 30, 2021
1 parent 4f02f52 commit ed4ae77
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 85 deletions.
10 changes: 5 additions & 5 deletions cmd/gateway/setup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func GatewayControllerParam(domain string, namespace string) ValidatorContext {
fields := strings.Split(param, "/")
l := len(fields)
if l != 3 {
return fmt.Errorf("unsupported path length, must be form DOMAIN/NAMESPACE/NAME")
return errors.New("unsupported path length, must be form DOMAIN/NAMESPACE/NAME")
}

for i := len(fields); i > 0; i-- {
Expand All @@ -55,7 +55,7 @@ func GatewayControllerParam(domain string, namespace string) ValidatorContext {
fields = fields[1:]
case 1:
if fields[0] == "" {
return fmt.Errorf("must provide a name")
return errors.New("must provide a name")
}
}
}
Expand Down Expand Up @@ -83,11 +83,11 @@ func MustValidateArguments(flagset *flag.FlagSet, validators ...ValidatorContext
msgs := ValidateArguments(flagset, validators...)
if msgs != nil {
for i := range msgs {
fmt.Printf("%s", msgs[i])
fmt.Fprintf(os.Stderr, "%s", msgs[i])
}
fmt.Println("")
fmt.Fprintln(os.Stderr, "")

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

os.Exit(1)
Expand Down
165 changes: 85 additions & 80 deletions cmd/gateway/setup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,35 @@ var _ = Describe("Main", func() {
}) // Generic Validator

Describe("CLI argument validation", func() {
type testCase struct {
Param string
Domain string
ExpError bool
}

var mockFlags *flag.FlagSet
var gatewayCtlrName string

tester := func(t testCase) {
err := mockFlags.Set(gatewayCtlrName, t.Param)
Expect(err).ToNot(HaveOccurred())

v := GatewayControllerParam(domain, t.Domain)
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
if t.ExpError {
Expect(err).To(HaveOccurred())
} else {
Expect(err).ToNot(HaveOccurred())
}
}
runner := func(table []testCase) {
for i := range table {
tester(table[i])
}
}

BeforeEach(func() {
domain = "k8s-gateway.nginx.org"
gatewayCtlrName = "gateway-ctlr-name"
Expand All @@ -136,97 +163,75 @@ var _ = Describe("Main", func() {
mockFlags = nil
})
It("should parse full gateway-ctlr-name", func() {
err := mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/nginx-gateway/my-gateway")
Expect(err).ToNot(HaveOccurred())

v := GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).ToNot(HaveOccurred())
t := testCase{
"k8s-gateway.nginx.org/nginx-gateway/my-gateway",
"nginx-gateway",
false,
}
tester(t)
}) // should parse full gateway-ctlr-name

It("should fail with too many path elements", func() {
err := mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/nginx-gateway/my-gateway/broken")
Expect(err).ToNot(HaveOccurred())

v := GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())
t := testCase{
"k8s-gateway.nginx.org/nginx-gateway/my-gateway/broken",
"nginx-gateway",
true,
}
tester(t)
}) // should fail with too many path elements

It("should fail with too few path elements", func() {
err := mockFlags.Set(gatewayCtlrName, "nginx-gateway/my-gateway")
Expect(err).ToNot(HaveOccurred())

v := GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())

err = mockFlags.Set(gatewayCtlrName, "my-gateway")
Expect(err).ToNot(HaveOccurred())

v = GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())
table := []testCase{
{
Param: "nginx-gateway/my-gateway",
Domain: "nginx-gateway",
ExpError: true,
},
{
Param: "my-gateway",
Domain: "nginx-gateway",
ExpError: true,
},
}

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())
runner(table)
}) // should fail with too few path elements

It("should verify constraints", func() {
// bad domain
err := mockFlags.Set(gatewayCtlrName, "invalid-domain/nginx-gateway/my-gateway")
Expect(err).ToNot(HaveOccurred())

v := GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())

// bad domain
err = mockFlags.Set(gatewayCtlrName, "/default/my-gateway")
Expect(err).ToNot(HaveOccurred())

v = GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())

// bad namespace
err = mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/default/my-gateway")
Expect(err).ToNot(HaveOccurred())

v = GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())

// bad namespace
err = mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org//my-gateway")
Expect(err).ToNot(HaveOccurred())

v = GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())

// bad name
err = mockFlags.Set(gatewayCtlrName, "k8s-gateway.nginx.org/default/")
Expect(err).ToNot(HaveOccurred())

v = GatewayControllerParam(domain, "nginx-gateway")
Expect(v.V).ToNot(BeNil())
table := []testCase{
{
// bad domain
Param: "invalid-domain/nginx-gateway/my-gateway",
Domain: "nginx-gateway",
ExpError: true,
},
{
// bad domain
Param: "/default/my-gateway",
Domain: "nginx-gateway",
ExpError: true,
},
{
// bad namespace
Param: "k8s-gateway.nginx.org/default/my-gateway",
Domain: "nginx-gateway",
ExpError: true,
},
{
// bad namespace
Param: "k8s-gateway.nginx.org//my-gateway",
Domain: "nginx-gateway",
ExpError: true,
},
{
// bad name
Param: "k8s-gateway.nginx.org/default/",
Domain: "nginx-gateway",
ExpError: true,
},
}

err = v.V(mockFlags)
Expect(err).To(HaveOccurred())
runner(table)
}) // should verify constraints
}) // CLI argument validation
}) // end Main

0 comments on commit ed4ae77

Please sign in to comment.