Merge pull request #28 from otaviof/issue-26

Fixing Pointer-to-String Flags and Improve Testing

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

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{},
  }, {

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{},
  },

pkg/shp/flags/buildrun_test.go

@@ -1,23 +1,88 @@
 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)
 
  name := "name"
  completeBuildRunSpec := buildv1alpha1.BuildRunSpec{
  ServiceAccount: &buildv1alpha1.ServiceAccount{Name: &name},
  Output: &buildv1alpha1.Image{
- Credentials: &v1.LocalObjectReference{Name: "name"},
+ Credentials: &corev1.LocalObjectReference{Name: "name"},
  Image: "image",
  },
  }

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