diff --git a/cmd/config/get_cluster.go b/cmd/config/get_cluster.go index 8ed4a273d..74d735691 100644 --- a/cmd/config/get_cluster.go +++ b/cmd/config/get_cluster.go @@ -103,7 +103,7 @@ func getClusters(out io.Writer, rootSettings *environment.AirshipCTLSettings) er fmt.Fprint(out, "No clusters found in the configuration.\n") } for _, cluster := range clusters { - fmt.Fprintf(out, "%s", cluster.PrettyString()) + fmt.Fprintf(out, "%s\n", cluster.PrettyString()) } return nil } diff --git a/cmd/config/get_cluster_test.go b/cmd/config/get_cluster_test.go index c99d08641..9158633fc 100644 --- a/cmd/config/get_cluster_test.go +++ b/cmd/config/get_cluster_test.go @@ -75,7 +75,7 @@ func TestGetAllClusters(t *testing.T) { clusters, err := conf.GetClusters() require.NoError(t, err) for _, cluster := range clusters { - expected += fmt.Sprintf("%s", cluster.PrettyString()) + expected += fmt.Sprintf("%s\n", cluster.PrettyString()) } test := getClusterTest{ diff --git a/pkg/config/config.go b/pkg/config/config.go index dfe27c2f2..1a1b88599 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -497,7 +497,7 @@ func (c *Config) Purge() error { func (c *Config) Equal(d *Config) bool { if d == nil { - return false + return d == c } clusterEq := reflect.DeepEqual(c.Clusters, d.Clusters) authInfoEq := reflect.DeepEqual(c.AuthInfos, d.AuthInfos) @@ -512,7 +512,7 @@ func (c *Config) Equal(d *Config) bool { // Cluster functions func (c *Cluster) Equal(d *Cluster) bool { if d == nil { - return false + return d == c } return c.NameInKubeconf == d.NameInKubeconf && c.Bootstrap == d.Bootstrap @@ -536,8 +536,8 @@ func (c *Cluster) PrettyString() string { clusterName := NewClusterComplexName() clusterName.FromName(c.NameInKubeconf) - return fmt.Sprintf("Cluster: %s\n%s:\n%s\n", - clusterName.ClusterName(), clusterName.ClusterType(), c.String()) + return fmt.Sprintf("Cluster: %s\n%s:\n%s", + clusterName.ClusterName(), clusterName.ClusterType(), c) } func (c *Cluster) KubeCluster() *kubeconfig.Cluster { @@ -550,11 +550,12 @@ func (c *Cluster) SetKubeCluster(kc *kubeconfig.Cluster) { // Context functions func (c *Context) Equal(d *Context) bool { if d == nil { - return false + return d == c } return c.NameInKubeconf == d.NameInKubeconf && c.Manifest == d.Manifest } + func (c *Context) String() string { yaml, err := yaml.Marshal(&c) if err != nil { @@ -566,7 +567,7 @@ func (c *Context) String() string { // AuthInfo functions func (c *AuthInfo) Equal(d *AuthInfo) bool { if d == nil { - return false + return d == c } return c == d } @@ -582,11 +583,12 @@ func (c *AuthInfo) String() string { // Manifest functions func (m *Manifest) Equal(n *Manifest) bool { if n == nil { - return false + return n == m } repositoryEq := reflect.DeepEqual(m.Repositories, n.Repositories) return repositoryEq && m.TargetPath == n.TargetPath } + func (m *Manifest) String() string { yaml, err := yaml.Marshal(&m) if err != nil { @@ -598,11 +600,16 @@ func (m *Manifest) String() string { // Repository functions func (r *Repository) Equal(s *Repository) bool { if s == nil { - return false + return r == s } - url := (r.Url == nil && s.Url == nil) || - (r.Url != nil && s.Url != nil && r.Url.String() == s.Url.String()) - return url && + var urlMatches bool + if r.Url != nil && s.Url != nil { + urlMatches = (r.Url.String() == s.Url.String()) + } else { + // this catches cases where one or both are nil + urlMatches = (r.Url == s.Url) + } + return urlMatches && r.Username == s.Username && r.TargetPath == s.TargetPath } @@ -616,8 +623,10 @@ func (r *Repository) String() string { // Modules functions func (m *Modules) Equal(n *Modules) bool { - - return n != nil && m.Dummy == n.Dummy + if n == nil { + return n == m + } + return m.Dummy == n.Dummy } func (m *Modules) String() string { yaml, err := yaml.Marshal(&m) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 451c8ee5a..864b5cfea 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -17,109 +17,133 @@ limitations under the License. package config import ( - "io/ioutil" + "fmt" "os" "path/filepath" - "reflect" - "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/client-go/tools/clientcmd" + + "opendev.org/airship/airshipctl/testutil" ) -// Testing related constants - -var AirshipStructs = [...]reflect.Value{ - reflect.ValueOf(DummyConfig()), - reflect.ValueOf(DummyCluster()), - reflect.ValueOf(DummyContext()), - reflect.ValueOf(DummyManifest()), - reflect.ValueOf(DummyAuthInfo()), - reflect.ValueOf(DummyRepository()), - reflect.ValueOf(DummyModules()), -} - -// I can probable reflect to generate this two slices, instead based on the 1st one -// Exercise left for later -- YES I will remove this comment in the next patchset -var AirshipStructsEqual = [...]reflect.Value{ - reflect.ValueOf(DummyConfig()), - reflect.ValueOf(DummyCluster()), - reflect.ValueOf(DummyContext()), - reflect.ValueOf(DummyManifest()), - reflect.ValueOf(DummyAuthInfo()), - reflect.ValueOf(DummyRepository()), - reflect.ValueOf(DummyModules()), -} - -var AirshipStructsDiff = [...]reflect.Value{ - reflect.ValueOf(NewConfig()), - reflect.ValueOf(NewCluster()), - reflect.ValueOf(NewContext()), - reflect.ValueOf(NewManifest()), - reflect.ValueOf(NewAuthInfo()), - reflect.ValueOf(NewRepository()), - reflect.ValueOf(NewModules()), -} - -// Test to complete min coverage func TestString(t *testing.T) { - for s := range AirshipStructs { - airStruct := AirshipStructs[s] - airStringMethod := airStruct.MethodByName("String") - yaml := airStringMethod.Call([]reflect.Value{}) - require.NotNil(t, yaml) + fSys := testutil.SetupTestFs(t, "testdata") - structName := strings.Split(airStruct.Type().String(), ".") - expectedFile := filepath.Join(testDataDir, "GoldenString", structName[1]+testMimeType) - expectedData, err := ioutil.ReadFile(expectedFile) - assert.Nil(t, err) - require.EqualValues(t, string(expectedData), yaml[0].String()) + tests := []struct { + name string + stringer fmt.Stringer + }{ + { + name: "config", + stringer: DummyConfig(), + }, + { + name: "context", + stringer: DummyContext(), + }, + { + name: "cluster", + stringer: DummyCluster(), + }, + { + name: "authinfo", + stringer: DummyAuthInfo(), + }, + { + name: "manifest", + stringer: DummyManifest(), + }, + { + name: "modules", + stringer: DummyModules(), + }, + { + name: "repository", + stringer: DummyRepository(), + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + filename := fmt.Sprintf("/%s-string.yaml", tt.name) + data, err := fSys.ReadFile(filename) + require.NoError(t, err) + + assert.Equal(t, string(data), tt.stringer.String()) + }) } } + func TestPrettyString(t *testing.T) { - conf := InitConfig(t) - cluster, err := conf.GetCluster("def", Ephemeral) + fSys := testutil.SetupTestFs(t, "testdata") + data, err := fSys.ReadFile("/prettycluster-string.yaml") require.NoError(t, err) - expectedFile := filepath.Join(testDataDir, "GoldenString", "PrettyCluster.yaml") - expectedData, err := ioutil.ReadFile(expectedFile) - assert.Nil(t, err) - - assert.EqualValues(t, cluster.PrettyString(), string(expectedData)) + cluster := DummyCluster() + assert.EqualValues(t, cluster.PrettyString(), string(data)) } func TestEqual(t *testing.T) { - for s := range AirshipStructs { - airStruct := AirshipStructs[s] - airStringMethod := airStruct.MethodByName("Equal") - args := []reflect.Value{AirshipStructsEqual[s]} - eq := airStringMethod.Call(args) - assert.NotNilf(t, eq, "Equal for %v failed to return response to Equal . ", airStruct.Type().String()) - require.Truef(t, eq[0].Bool(), "Equal for %v failed to return true for equal values ", airStruct.Type().String()) + t.Run("config-equal", func(t *testing.T) { + testConfig1 := NewConfig() + testConfig2 := NewConfig() + testConfig2.Kind = "Different" + assert.True(t, testConfig1.Equal(testConfig1)) + assert.False(t, testConfig1.Equal(testConfig2)) + assert.False(t, testConfig1.Equal(nil)) + }) - // Lets test Equals against nil struct - args = []reflect.Value{reflect.New(airStruct.Type()).Elem()} - nileq := airStringMethod.Call(args) - assert.NotNil(t, nileq, "Equal for %v failed to return response to Equal . ", airStruct.Type().String()) - require.Falsef(t, nileq[0].Bool(), - "Equal for %v failed to return false when comparing against nil value ", airStruct.Type().String()) + t.Run("cluster-equal", func(t *testing.T) { + testCluster1 := &Cluster{NameInKubeconf: "same"} + testCluster2 := &Cluster{NameInKubeconf: "different"} + assert.True(t, testCluster1.Equal(testCluster1)) + assert.False(t, testCluster1.Equal(testCluster2)) + assert.False(t, testCluster1.Equal(nil)) + }) - // Ignore False Equals test for AuthInfo for now - if airStruct.Type().String() == "*config.AuthInfo" { - continue - } - // Lets test that equal returns false when they are diff - args = []reflect.Value{AirshipStructsDiff[s]} - neq := airStringMethod.Call(args) - assert.NotNil(t, neq, "Equal for %v failed to return response to Equal . ", airStruct.Type().String()) - require.Falsef(t, neq[0].Bool(), - "Equal for %v failed to return false for different values ", airStruct.Type().String()) + t.Run("context-equal", func(t *testing.T) { + testContext1 := &Context{NameInKubeconf: "same"} + testContext2 := &Context{NameInKubeconf: "different"} + assert.True(t, testContext1.Equal(testContext1)) + assert.False(t, testContext1.Equal(testContext2)) + assert.False(t, testContext1.Equal(nil)) + }) - } + // TODO(howell): this needs to be fleshed out when the AuthInfo type is finished + t.Run("authinfo-equal", func(t *testing.T) { + testAuthInfo1 := &AuthInfo{} + assert.True(t, testAuthInfo1.Equal(testAuthInfo1)) + assert.False(t, testAuthInfo1.Equal(nil)) + }) + + t.Run("manifest-equal", func(t *testing.T) { + testManifest1 := &Manifest{TargetPath: "same"} + testManifest2 := &Manifest{TargetPath: "different"} + assert.True(t, testManifest1.Equal(testManifest1)) + assert.False(t, testManifest1.Equal(testManifest2)) + assert.False(t, testManifest1.Equal(nil)) + }) + + t.Run("repository-equal", func(t *testing.T) { + testRepository1 := &Repository{TargetPath: "same"} + testRepository2 := &Repository{TargetPath: "different"} + assert.True(t, testRepository1.Equal(testRepository1)) + assert.False(t, testRepository1.Equal(testRepository2)) + assert.False(t, testRepository1.Equal(nil)) + }) + + // TODO(howell): this needs to be fleshed out when the Modules type is finished + t.Run("modules-equal", func(t *testing.T) { + testModules1 := &Modules{Dummy: "same"} + testModules2 := &Modules{Dummy: "different"} + assert.True(t, testModules1.Equal(testModules1)) + assert.False(t, testModules1.Equal(testModules2)) + assert.False(t, testModules1.Equal(nil)) + }) } func TestLoadConfig(t *testing.T) { diff --git a/pkg/config/testdata/GoldenString/PrettyCluster.yaml b/pkg/config/testdata/GoldenString/PrettyCluster.yaml deleted file mode 100644 index 056b235e6..000000000 --- a/pkg/config/testdata/GoldenString/PrettyCluster.yaml +++ /dev/null @@ -1,9 +0,0 @@ -Cluster: def -ephemeral: -bootstrap-info: "" -cluster-kubeconf: def_ephemeral - -LocationOfOrigin: ../../pkg/config/testdata/kubeconfig.yaml -insecure-skip-tls-verify: true -server: http://5.6.7.8 - diff --git a/pkg/config/testdata/GoldenString/AuthInfo.yaml b/pkg/config/testdata/authinfo-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/AuthInfo.yaml rename to pkg/config/testdata/authinfo-string.yaml diff --git a/pkg/config/testdata/GoldenString/Cluster.yaml b/pkg/config/testdata/cluster-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Cluster.yaml rename to pkg/config/testdata/cluster-string.yaml diff --git a/pkg/config/testdata/GoldenString/Config.yaml b/pkg/config/testdata/config-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Config.yaml rename to pkg/config/testdata/config-string.yaml diff --git a/pkg/config/testdata/GoldenString/Context.yaml b/pkg/config/testdata/context-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Context.yaml rename to pkg/config/testdata/context-string.yaml diff --git a/pkg/config/testdata/GoldenString/Garbage.yaml b/pkg/config/testdata/garbage-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Garbage.yaml rename to pkg/config/testdata/garbage-string.yaml diff --git a/pkg/config/testdata/GoldenString/Manifest.yaml b/pkg/config/testdata/manifest-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Manifest.yaml rename to pkg/config/testdata/manifest-string.yaml diff --git a/pkg/config/testdata/GoldenString/Modules.yaml b/pkg/config/testdata/modules-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Modules.yaml rename to pkg/config/testdata/modules-string.yaml diff --git a/pkg/config/testdata/prettycluster-string.yaml b/pkg/config/testdata/prettycluster-string.yaml new file mode 100644 index 000000000..1142fcf76 --- /dev/null +++ b/pkg/config/testdata/prettycluster-string.yaml @@ -0,0 +1,8 @@ +Cluster: dummycluster +target: +bootstrap-info: dummy_bootstrap +cluster-kubeconf: dummycluster_target + +LocationOfOrigin: "" +certificate-authority: dummy_ca +server: http://dummy.server diff --git a/pkg/config/testdata/GoldenString/Repository.yaml b/pkg/config/testdata/repository-string.yaml similarity index 100% rename from pkg/config/testdata/GoldenString/Repository.yaml rename to pkg/config/testdata/repository-string.yaml