From 6a1bf8eaaf85d05a57aaa7828be87095586fbfab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ot=C3=A1vio=20Fernandes?= Date: Fri, 25 Jun 2021 10:06:12 +0200 Subject: [PATCH] Testing Flags Package Covering the flags generate by this package with unit-testing, so every command-line flag is checked. Also, adding unit-tests on StrategyKindValue and StringPointerValue. --- pkg/shp/flags/build.go | 14 +-- pkg/shp/flags/build_test.go | 114 +++++++++++++++++++-- pkg/shp/flags/buildrun.go | 9 +- pkg/shp/flags/buildrun_test.go | 69 ++++++++++++- pkg/shp/flags/doc.go | 2 +- pkg/shp/flags/flags.go | 65 ++++++++---- pkg/shp/flags/strategy_kind_value.go | 12 +-- pkg/shp/flags/strategy_kind_value_test.go | 24 +++++ pkg/shp/flags/string_pointer_value_test.go | 28 +++++ 9 files changed, 290 insertions(+), 47 deletions(-) create mode 100644 pkg/shp/flags/strategy_kind_value_test.go create mode 100644 pkg/shp/flags/string_pointer_value_test.go diff --git a/pkg/shp/flags/build.go b/pkg/shp/flags/build.go index 88cc831b4..e379debc2 100644 --- a/pkg/shp/flags/build.go +++ b/pkg/shp/flags/build.go @@ -10,15 +10,18 @@ import ( // BuildSpecFromFlags creates a BuildSpec instance based on command-line flags. func BuildSpecFromFlags(flags *pflag.FlagSet) *buildv1alpha1.BuildSpec { clusterBuildStrategyKind := buildv1alpha1.ClusterBuildStrategyKind + empty := "" spec := &buildv1alpha1.BuildSpec{ Source: buildv1alpha1.Source{ Credentials: &corev1.LocalObjectReference{}, + Revision: &empty, + ContextDir: &empty, }, Strategy: &buildv1alpha1.Strategy{ Kind: &clusterBuildStrategyKind, APIVersion: buildv1alpha1.SchemeGroupVersion.Version, }, - Dockerfile: nil, + Dockerfile: &empty, Builder: &buildv1alpha1.Image{ Credentials: &corev1.LocalObjectReference{}, }, @@ -30,16 +33,9 @@ func BuildSpecFromFlags(flags *pflag.FlagSet) *buildv1alpha1.BuildSpec { sourceFlags(flags, &spec.Source) strategyFlags(flags, spec.Strategy) - - flags.Var( - NewStringPointerValue(spec.Dockerfile), - "dockerfile", - "path to dockerfile relative to repository", - ) - + dockerfileFlags(flags, spec.Dockerfile) imageFlags(flags, "builder", spec.Builder) imageFlags(flags, "output", &spec.Output) - timeoutFlags(flags, spec.Timeout) return spec diff --git a/pkg/shp/flags/build_test.go b/pkg/shp/flags/build_test.go index db2cef373..ba0b2b054 100644 --- a/pkg/shp/flags/build_test.go +++ b/pkg/shp/flags/build_test.go @@ -2,22 +2,124 @@ package flags import ( "testing" + "time" "github.com/onsi/gomega" - o "github.com/onsi/gomega" buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" - v1 "k8s.io/api/core/v1" + "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + o "github.com/onsi/gomega" ) +func TestBuildSpecFromFlags(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + str := "something-random" + credentials := corev1.LocalObjectReference{Name: "name"} + buildStrategyKind := buildv1alpha1.ClusterBuildStrategyKind + expected := &buildv1alpha1.BuildSpec{ + Source: buildv1alpha1.Source{ + Credentials: &credentials, + URL: str, + Revision: &str, + ContextDir: &str, + }, + Strategy: &buildv1alpha1.Strategy{ + Name: str, + Kind: &buildStrategyKind, + APIVersion: buildv1alpha1.SchemeGroupVersion.Version, + }, + Dockerfile: &str, + Builder: &buildv1alpha1.Image{ + Credentials: &credentials, + Image: str, + }, + Output: buildv1alpha1.Image{ + Credentials: &credentials, + Image: str, + }, + Timeout: &metav1.Duration{ + Duration: 1 * time.Second, + }, + } + + cmd := &cobra.Command{} + flags := cmd.PersistentFlags() + spec := BuildSpecFromFlags(flags) + + t.Run(".spec.source", func(t *testing.T) { + err := flags.Set(SourceURLFlag, expected.Source.URL) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(SourceRevisionFlag, *expected.Source.Revision) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(SourceCredentialsSecretFlag, expected.Source.Credentials.Name) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(StrategyAPIVersionFlag, expected.Strategy.APIVersion) + g.Expect(err).To(o.BeNil()) + + g.Expect(expected.Source).To(o.Equal(spec.Source), "spec.source") + }) + + t.Run(".spec.strategy", func(t *testing.T) { + err := flags.Set(StrategyKindFlag, string(buildv1alpha1.ClusterBuildStrategyKind)) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(StrategyNameFlag, expected.Strategy.Name) + g.Expect(err).To(o.BeNil()) + + g.Expect(expected.Strategy).To(o.Equal(spec.Strategy), "spec.strategy") + }) + + t.Run(".spec.dockerfile", func(t *testing.T) { + err := flags.Set(DockerfileFlag, *expected.Dockerfile) + g.Expect(err).To(o.BeNil()) + + g.Expect(spec.Dockerfile).NotTo(o.BeNil()) + g.Expect(*expected.Dockerfile).To(o.Equal(*spec.Dockerfile), "spec.dockerfile") + }) + + t.Run(".spec.builder", func(t *testing.T) { + err := flags.Set(BuilderImageFlag, expected.Builder.Image) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(BuilderCredentialsSecretFlag, expected.Builder.Credentials.Name) + g.Expect(err).To(o.BeNil()) + + g.Expect(*expected.Builder).To(o.Equal(*spec.Builder), "spec.builder") + }) + + t.Run(".spec.output", func(t *testing.T) { + err := flags.Set(OutputImageFlag, expected.Output.Image) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(OutputCredentialsSecretFlag, expected.Output.Credentials.Name) + g.Expect(err).To(o.BeNil()) + + g.Expect(expected.Output).To(o.Equal(spec.Output), "spec.output") + }) + + t.Run(".spec.timeout", func(t *testing.T) { + err := flags.Set(TimeoutFlag, expected.Timeout.Duration.String()) + g.Expect(err).To(o.BeNil()) + + g.Expect(*expected.Timeout).To(o.Equal(*spec.Timeout), "spec.timeout") + }) +} + func TestSanitizeBuildSpec(t *testing.T) { g := gomega.NewGomegaWithT(t) completeBuildSpec := buildv1alpha1.BuildSpec{ Source: buildv1alpha1.Source{ - Credentials: &v1.LocalObjectReference{Name: "name"}, + Credentials: &corev1.LocalObjectReference{Name: "name"}, }, Builder: &buildv1alpha1.Image{ - Credentials: &v1.LocalObjectReference{Name: "name"}, + Credentials: &corev1.LocalObjectReference{Name: "name"}, Image: "image", }, } @@ -33,13 +135,13 @@ func TestSanitizeBuildSpec(t *testing.T) { }, { name: "should clean-up `.spec.source.credentials`", in: buildv1alpha1.BuildSpec{Source: buildv1alpha1.Source{ - Credentials: &v1.LocalObjectReference{}, + Credentials: &corev1.LocalObjectReference{}, }}, out: buildv1alpha1.BuildSpec{}, }, { name: "should clean-up `.spec.builder.credentials`", in: buildv1alpha1.BuildSpec{Builder: &buildv1alpha1.Image{ - Credentials: &v1.LocalObjectReference{}, + Credentials: &corev1.LocalObjectReference{}, }}, out: buildv1alpha1.BuildSpec{}, }, { diff --git a/pkg/shp/flags/buildrun.go b/pkg/shp/flags/buildrun.go index 92ab5c329..fff124ca9 100644 --- a/pkg/shp/flags/buildrun.go +++ b/pkg/shp/flags/buildrun.go @@ -10,10 +10,13 @@ import ( // BuildRunSpecFromFlags creates a BuildRun spec from command-line flags. func BuildRunSpecFromFlags(flags *pflag.FlagSet) *buildv1alpha1.BuildRunSpec { + empty := "" spec := &buildv1alpha1.BuildRunSpec{ - BuildRef: &buildv1alpha1.BuildRef{}, - ServiceAccount: &buildv1alpha1.ServiceAccount{}, - Timeout: &metav1.Duration{}, + BuildRef: &buildv1alpha1.BuildRef{}, + ServiceAccount: &buildv1alpha1.ServiceAccount{ + Name: &empty, + }, + Timeout: &metav1.Duration{}, Output: &buildv1alpha1.Image{ Credentials: &corev1.LocalObjectReference{}, }, diff --git a/pkg/shp/flags/buildrun_test.go b/pkg/shp/flags/buildrun_test.go index 22cdc32ad..9810e3a78 100644 --- a/pkg/shp/flags/buildrun_test.go +++ b/pkg/shp/flags/buildrun_test.go @@ -1,15 +1,80 @@ package flags import ( + "fmt" "testing" + "time" "github.com/onsi/gomega" buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" - v1 "k8s.io/api/core/v1" + "github.com/spf13/cobra" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" o "github.com/onsi/gomega" ) +func TestBuildRunSpecFromFlags(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + str := "something-random" + expected := &buildv1alpha1.BuildRunSpec{ + BuildRef: &buildv1alpha1.BuildRef{ + Name: str, + }, + ServiceAccount: &buildv1alpha1.ServiceAccount{ + Name: &str, + Generate: false, + }, + Timeout: &metav1.Duration{ + Duration: 1 * time.Second, + }, + Output: &buildv1alpha1.Image{ + Credentials: &corev1.LocalObjectReference{Name: "name"}, + Image: str, + }, + } + + cmd := &cobra.Command{} + flags := cmd.PersistentFlags() + spec := BuildRunSpecFromFlags(flags) + + t.Run(".spec.buildRef", func(t *testing.T) { + err := flags.Set(BuildrefNameFlag, expected.BuildRef.Name) + g.Expect(err).To(o.BeNil()) + + g.Expect(*expected.BuildRef).To(o.Equal(*spec.BuildRef), "spec.buildRef") + }) + + t.Run(".spec.serviceAccount", func(t *testing.T) { + err := flags.Set(ServiceAccountNameFlag, *expected.ServiceAccount.Name) + g.Expect(err).To(o.BeNil()) + + generate := fmt.Sprintf("%v", expected.ServiceAccount.Generate) + err = flags.Set(ServiceAccountGenerateFlag, generate) + g.Expect(err).To(o.BeNil()) + + g.Expect(*expected.ServiceAccount).To(o.Equal(*spec.ServiceAccount), "spec.serviceAccount") + }) + + t.Run(".spec.timeout", func(t *testing.T) { + err := flags.Set(TimeoutFlag, expected.Timeout.Duration.String()) + g.Expect(err).To(o.BeNil()) + + g.Expect(*expected.Timeout).To(o.Equal(*spec.Timeout), "spec.timeout") + }) + + t.Run(".spec.output", func(t *testing.T) { + err := flags.Set(OutputImageFlag, expected.Output.Image) + g.Expect(err).To(o.BeNil()) + + err = flags.Set(OutputCredentialsSecretFlag, expected.Output.Credentials.Name) + g.Expect(err).To(o.BeNil()) + + g.Expect(*expected.Output).To(o.Equal(*spec.Output), "spec.output") + }) +} + func TestSanitizeBuildRunSpec(t *testing.T) { g := gomega.NewGomegaWithT(t) @@ -17,7 +82,7 @@ func TestSanitizeBuildRunSpec(t *testing.T) { completeBuildRunSpec := buildv1alpha1.BuildRunSpec{ ServiceAccount: &buildv1alpha1.ServiceAccount{Name: &name}, Output: &buildv1alpha1.Image{ - Credentials: &v1.LocalObjectReference{Name: "name"}, + Credentials: &corev1.LocalObjectReference{Name: "name"}, Image: "image", }, } diff --git a/pkg/shp/flags/doc.go b/pkg/shp/flags/doc.go index 96e30c91c..f943025c0 100644 --- a/pkg/shp/flags/doc.go +++ b/pkg/shp/flags/doc.go @@ -4,7 +4,7 @@ // For instance: // // cmd := &cobra.Command{} -// br := flags.BuildRunSpecFlags(cmd.Flags()) +// br := flags.BuildRunSpecFromFlags(cmd.Flags()) // flags.SanitizeBuildRunSpec(&br.Spec) // // The snippet above shows how to decorate an existing cobra.Command instance with flags, and return diff --git a/pkg/shp/flags/flags.go b/pkg/shp/flags/flags.go index 91c16ca98..cbdd008a7 100644 --- a/pkg/shp/flags/flags.go +++ b/pkg/shp/flags/flags.go @@ -10,9 +10,22 @@ import ( ) const ( - BuildrefNameFlag = "buildref-name" - SourceURLFlag = "source-url" - OutputImageFlag = "output-image" + BuildrefNameFlag = "buildref-name" + BuilderImageFlag = "builder-image" + BuilderCredentialsSecretFlag = "builder-credentials-secret" + DockerfileFlag = "dockerfile" + SourceURLFlag = "source-url" + SourceRevisionFlag = "source-revision" + SourceContextDirFlag = "source-context-dir" + SourceCredentialsSecretFlag = "source-credentials-secret" + StrategyAPIVersionFlag = "strategy-apiversion" + StrategyKindFlag = "strategy-kind" + StrategyNameFlag = "strategy-name" + OutputImageFlag = "output-image" + OutputCredentialsSecretFlag = "output-credentials-secret" + ServiceAccountNameFlag = "sa-name" + ServiceAccountGenerateFlag = "sa-generate" + TimeoutFlag = "timeout" ) // sourceFlags flags for ".spec.source" @@ -23,19 +36,21 @@ func sourceFlags(flags *pflag.FlagSet, source *buildv1alpha1.Source) { "", "git repository source URL", ) - flags.Var( - NewStringPointerValue(source.Revision), - "source-revision", + flags.StringVar( + source.Revision, + SourceRevisionFlag, + "", "git repository source revision", ) - flags.Var( - NewStringPointerValue(source.ContextDir), - "source-context-dir", + flags.StringVar( + source.ContextDir, + SourceContextDirFlag, + "", "use a inner directory as context directory", ) flags.StringVar( &source.Credentials.Name, - "source-credentials-secret", + SourceCredentialsSecretFlag, "", "name of the secret with git repository credentials", ) @@ -45,18 +60,18 @@ func sourceFlags(flags *pflag.FlagSet, source *buildv1alpha1.Source) { func strategyFlags(flags *pflag.FlagSet, strategy *buildv1alpha1.Strategy) { flags.StringVar( &strategy.APIVersion, - "strategy-apiversion", + StrategyAPIVersionFlag, buildv1alpha1.SchemeGroupVersion.Version, "kubernetes api-version of the build-strategy resource", ) flags.Var( NewStrategyKindValue(strategy.Kind), - "strategy-kind", + StrategyKindFlag, "build-strategy kind", ) flags.StringVar( &strategy.Name, - "strategy-name", + StrategyNameFlag, "buildpacks-v3", "build-strategy name", ) @@ -78,11 +93,20 @@ func imageFlags(flags *pflag.FlagSet, prefix string, image *buildv1alpha1.Image) ) } +// dockerfileFlags register dockerfile flag as pointer to string. +func dockerfileFlags(flags *pflag.FlagSet, dockerfile *string) { + flags.Var( + NewStringPointerValue(dockerfile), + DockerfileFlag, + "path to dockerfile relative to repository", + ) +} + // timeoutFlags register a timeout flag as time.Duration instance. func timeoutFlags(flags *pflag.FlagSet, timeout *metav1.Duration) { flags.DurationVar( &timeout.Duration, - "timeout", + TimeoutFlag, time.Duration(0), "build process timeout", ) @@ -106,15 +130,16 @@ func buildRefFlags(flags *pflag.FlagSet, buildRef *buildv1alpha1.BuildRef) { // serviceAccountFlags register flags for BuildRun's spec.serviceAccount attribute. func serviceAccountFlags(flags *pflag.FlagSet, sa *buildv1alpha1.ServiceAccount) { - flags.Var( - NewStringPointerValue(sa.Name), - "sa-name", - "service-account name", + flags.StringVar( + sa.Name, + ServiceAccountNameFlag, + "", + "Kubernetes service-account name", ) flags.BoolVar( &sa.Generate, - "sa-generate", + ServiceAccountGenerateFlag, false, - "generate a service-account for the build", + "generate a Kubernetes service-account for the build", ) } diff --git a/pkg/shp/flags/strategy_kind_value.go b/pkg/shp/flags/strategy_kind_value.go index 712e2f849..41fbd1bb3 100644 --- a/pkg/shp/flags/strategy_kind_value.go +++ b/pkg/shp/flags/strategy_kind_value.go @@ -21,13 +21,13 @@ func (s *StrategyKindValue) String() string { } // Set set the informed string as BuildStrategyKind by casting. -func (s *StrategyKindValue) Set(str string) error { - var strInterface interface{} = str - var ok bool - s.kindPtr, ok = strInterface.(*buildv1alpha1.BuildStrategyKind) - if !ok { - return fmt.Errorf("unable to cast '%s' as BuildStrategyKind value", str) +func (s *StrategyKindValue) Set(value string) error { + kind := buildv1alpha1.BuildStrategyKind(value) + if kind != buildv1alpha1.NamespacedBuildStrategyKind && + kind != buildv1alpha1.ClusterBuildStrategyKind { + return fmt.Errorf("'%s' is an invalid BuildStrategyKind", value) } + *s.kindPtr = kind return nil } diff --git a/pkg/shp/flags/strategy_kind_value_test.go b/pkg/shp/flags/strategy_kind_value_test.go new file mode 100644 index 000000000..e4ec12d2a --- /dev/null +++ b/pkg/shp/flags/strategy_kind_value_test.go @@ -0,0 +1,24 @@ +package flags + +import ( + "testing" + + "github.com/onsi/gomega" + o "github.com/onsi/gomega" + buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" +) + +func TestStrategyKindValue(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + buildStrategyKind := buildv1alpha1.ClusterBuildStrategyKind + v := NewStrategyKindValue(&buildStrategyKind) + + expected := buildv1alpha1.NamespacedBuildStrategyKind + + err := v.Set(string(expected)) + g.Expect(err).To(o.BeNil()) + + g.Expect(string(expected)).To(o.Equal(v.String())) + g.Expect(expected).To(o.Equal(buildStrategyKind)) +} diff --git a/pkg/shp/flags/string_pointer_value_test.go b/pkg/shp/flags/string_pointer_value_test.go new file mode 100644 index 000000000..a11177631 --- /dev/null +++ b/pkg/shp/flags/string_pointer_value_test.go @@ -0,0 +1,28 @@ +package flags + +import ( + "testing" + + "github.com/onsi/gomega" + o "github.com/onsi/gomega" + "github.com/spf13/cobra" +) + +func TestStringPointerValue(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + flagName := "flag" + value := "value" + targetStr := "string" + + cmd := &cobra.Command{} + flags := cmd.PersistentFlags() + flags.Var(NewStringPointerValue(&targetStr), flagName, "") + + err := flags.Set(flagName, value) + g.Expect(err).To(o.BeNil()) + + v, err := flags.GetString(flagName) + g.Expect(err).To(o.BeNil()) + g.Expect(value).To(o.Equal(v)) +}