From 1d90e34e7194b2873667b9e81a7102ecdd5b54e0 Mon Sep 17 00:00:00 2001 From: chansuke Date: Wed, 27 Mar 2024 15:30:48 +0900 Subject: [PATCH 1/3] fix: return error instead of log.Fatalf() --- .../namereferencetransformer_test.go | 6 +- .../accumulator/resaccumulator_test.go | 33 ++-- .../plugins/execplugin/execplugin_test.go | 5 +- api/internal/plugins/utils/utils_test.go | 5 +- api/internal/target/kusttarget_test.go | 186 +++++++++--------- api/resmap/reswrangler_test.go | 56 +++--- api/resource/factory.go | 16 +- api/resource/factory_test.go | 18 +- api/resource/idset_test.go | 40 +++- api/resource/resource_test.go | 81 +++++--- api/testutils/resmaptest/rmbuilder.go | 20 +- 11 files changed, 282 insertions(+), 184 deletions(-) diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index c964c5f9ee..d54310c4eb 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -590,7 +590,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { rf := provider.NewDefaultDepProvider().GetResourceFactory() - v1 := rf.FromMapWithName( + v1, _ := rf.FromMapWithName( "volume1", map[string]interface{}{ "apiVersion": "v1", @@ -599,7 +599,7 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "name": "someprefix-volume1", }, }) - c1 := rf.FromMapWithName( + c1, _ := rf.FromMapWithName( "claim1", map[string]interface{}{ "apiVersion": "v1", @@ -614,7 +614,7 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { }) v2 := v1.DeepCopy() - c2 := rf.FromMapWithName( + c2, _ := rf.FromMapWithName( "claim1", map[string]interface{}{ "apiVersion": "v1", diff --git a/api/internal/accumulator/resaccumulator_test.go b/api/internal/accumulator/resaccumulator_test.go index 6cc5656824..5c3263c51d 100644 --- a/api/internal/accumulator/resaccumulator_test.go +++ b/api/internal/accumulator/resaccumulator_test.go @@ -143,16 +143,20 @@ func expectLog(t *testing.T, log bytes.Buffer, expect string) { func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { ra := makeResAccumulator(t) rm0 := resmap.New() - err := rm0.Append( - provider.NewDefaultDepProvider().GetResourceFactory().FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Service", - "metadata": map[string]interface{}{ - "name": "backendOne", - "namespace": "fooNamespace", - }, - })) + + r, err := provider.NewDefaultDepProvider().GetResourceFactory().FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "Service", + "metadata": map[string]interface{}{ + "name": "backendOne", + "namespace": "fooNamespace", + }, + }) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + err = rm0.Append(r) if err != nil { t.Fatalf("unexpected err: %v", err) } @@ -227,7 +231,8 @@ func TestResolveVarConflicts(t *testing.T) { // create accumulators holding apparently conflicting vars that are not // actually in conflict because they point to the same concrete value. rm0 := resmap.New() - err := rm0.Append(rf.FromMap(fooAws)) + r0, _ := rf.FromMap(fooAws) + err := rm0.Append(r0) require.NoError(t, err) ac0 := MakeEmptyAccumulator() err = ac0.AppendAll(rm0) @@ -236,7 +241,8 @@ func TestResolveVarConflicts(t *testing.T) { require.NoError(t, err) rm1 := resmap.New() - err = rm1.Append(rf.FromMap(barAws)) + r1, _ := rf.FromMap(barAws) + err = rm1.Append(r1) require.NoError(t, err) ac1 := MakeEmptyAccumulator() err = ac1.AppendAll(rm1) @@ -255,7 +261,8 @@ func TestResolveVarConflicts(t *testing.T) { // two above (because it contains a variable whose name is used in the other // accumulators AND whose concrete values are different). rm2 := resmap.New() - err = rm2.Append(rf.FromMap(barGcp)) + r2, _ := rf.FromMap(barGcp) + err = rm2.Append(r2) require.NoError(t, err) ac2 := MakeEmptyAccumulator() err = ac2.AppendAll(rm2) diff --git a/api/internal/plugins/execplugin/execplugin_test.go b/api/internal/plugins/execplugin/execplugin_test.go index 6c9ff3b882..c21d20acf7 100644 --- a/api/internal/plugins/execplugin/execplugin_test.go +++ b/api/internal/plugins/execplugin/execplugin_test.go @@ -41,7 +41,7 @@ s/$BAR/bar baz/g } pvd := provider.NewDefaultDepProvider() rf := resmap.NewFactory(pvd.GetResourceFactory()) - pluginConfig := rf.RF().FromMap( + pluginConfig, err := rf.RF().FromMap( map[string]interface{}{ "apiVersion": "someteam.example.com/v1", "kind": "SedTransformer", @@ -51,6 +51,9 @@ s/$BAR/bar baz/g "argsOneLiner": "one two 'foo bar'", "argsFromFile": "sed-input.txt", }) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } pluginConfig.RemoveBuildAnnotations() pc := types.DisabledPluginConfig() diff --git a/api/internal/plugins/utils/utils_test.go b/api/internal/plugins/utils/utils_test.go index 90d7a50f15..d1ad843013 100644 --- a/api/internal/plugins/utils/utils_test.go +++ b/api/internal/plugins/utils/utils_test.go @@ -32,11 +32,14 @@ func TestDeterminePluginSrcRoot(t *testing.T) { } func makeConfigMap(rf *resource.Factory, name, behavior string, hashValue *string) *resource.Resource { - r := rf.FromMap(map[string]interface{}{ + r, err := rf.FromMap(map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", "metadata": map[string]interface{}{"name": name}, }) + if err != nil { + panic(err) + } annotations := map[string]string{} if behavior != "" { annotations[BehaviorAnnotation] = behavior diff --git a/api/internal/target/kusttarget_test.go b/api/internal/target/kusttarget_test.go index b14a8bfbe6..5dfb086693 100644 --- a/api/internal/target/kusttarget_test.go +++ b/api/internal/target/kusttarget_test.go @@ -190,12 +190,58 @@ metadata: pvd := provider.NewDefaultDepProvider() resFactory := pvd.GetResourceFactory() - resources := []*resource.Resource{ - resFactory.FromMapWithName("dply1", map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", + r0, _ := resFactory.FromMapWithName("dply1", map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo-dply1-bar", + "namespace": "ns1", + "labels": map[string]interface{}{ + "app": "nginx", + }, + "annotations": map[string]interface{}{ + "note": "This is a test annotation", + }, + }, + "spec": map[string]interface{}{ + "replica": "3", + "selector": map[string]interface{}{ + "matchLabels": map[string]interface{}{ + "app": "nginx", + }, + }, + "template": map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": map[string]interface{}{ + "note": "This is a test annotation", + }, + "labels": map[string]interface{}{ + "app": "nginx", + }, + }, + }, + }, + }) + r1, _ := resFactory.FromMapWithName("ns1", map[string]interface{}{ + "apiVersion": "v1", + "kind": "Namespace", + "metadata": map[string]interface{}{ + "name": "ns1", + "labels": map[string]interface{}{ + "app": "nginx", + }, + "annotations": map[string]interface{}{ + "note": "This is a test annotation", + }, + }, + }) + + r2, _ := resFactory.FromMapWithName("literalConfigMap", + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", "metadata": map[string]interface{}{ - "name": "foo-dply1-bar", + "name": "foo-literalConfigMap-bar-g5f6t456f5", "namespace": "ns1", "labels": map[string]interface{}{ "app": "nginx", @@ -204,30 +250,19 @@ metadata: "note": "This is a test annotation", }, }, - "spec": map[string]interface{}{ - "replica": "3", - "selector": map[string]interface{}{ - "matchLabels": map[string]interface{}{ - "app": "nginx", - }, - }, - "template": map[string]interface{}{ - "metadata": map[string]interface{}{ - "annotations": map[string]interface{}{ - "note": "This is a test annotation", - }, - "labels": map[string]interface{}{ - "app": "nginx", - }, - }, - }, + "data": map[string]interface{}{ + "DB_USERNAME": "admin", + "DB_PASSWORD": "somepw", }, - }), - resFactory.FromMapWithName("ns1", map[string]interface{}{ + }) + + r3, _ := resFactory.FromMapWithName("secret", + map[string]interface{}{ "apiVersion": "v1", - "kind": "Namespace", + "kind": "Secret", "metadata": map[string]interface{}{ - "name": "ns1", + "name": "foo-secret-bar-82c2g5f8f6", + "namespace": "ns1", "labels": map[string]interface{}{ "app": "nginx", }, @@ -235,47 +270,14 @@ metadata: "note": "This is a test annotation", }, }, - }), - resFactory.FromMapWithName("literalConfigMap", - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "foo-literalConfigMap-bar-g5f6t456f5", - "namespace": "ns1", - "labels": map[string]interface{}{ - "app": "nginx", - }, - "annotations": map[string]interface{}{ - "note": "This is a test annotation", - }, - }, - "data": map[string]interface{}{ - "DB_USERNAME": "admin", - "DB_PASSWORD": "somepw", - }, - }), - resFactory.FromMapWithName("secret", - map[string]interface{}{ - "apiVersion": "v1", - "kind": "Secret", - "metadata": map[string]interface{}{ - "name": "foo-secret-bar-82c2g5f8f6", - "namespace": "ns1", - "labels": map[string]interface{}{ - "app": "nginx", - }, - "annotations": map[string]interface{}{ - "note": "This is a test annotation", - }, - }, - "type": ifc.SecretTypeOpaque, - "data": map[string]interface{}{ - "DB_USERNAME": base64.StdEncoding.EncodeToString([]byte("admin")), - "DB_PASSWORD": base64.StdEncoding.EncodeToString([]byte("somepw")), - }, - }), - } + "type": ifc.SecretTypeOpaque, + "data": map[string]interface{}{ + "DB_USERNAME": base64.StdEncoding.EncodeToString([]byte("admin")), + "DB_PASSWORD": base64.StdEncoding.EncodeToString([]byte("somepw")), + }, + }) + + resources := []*resource.Resource{r0, r1, r2, r3} expected := resmap.New() for _, r := range resources { @@ -352,31 +354,31 @@ metadata: pvd := provider.NewDefaultDepProvider() resFactory := pvd.GetResourceFactory() - resources := []*resource.Resource{ - resFactory.FromMapWithName("deployment1", map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "foo-deployment1-bar", - "namespace": "ns1", - }, - }), resFactory.FromMapWithName("config", map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "config-bar", - "namespace": "ns1", - }, - }), resFactory.FromMapWithName("secret", map[string]interface{}{ - "apiVersion": "v1", - "kind": "Secret", - "metadata": map[string]interface{}{ - "name": "foo-secret", - "namespace": "ns1", - }, - }), - } - + r0, _ := resFactory.FromMapWithName("deployment1", map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "foo-deployment1-bar", + "namespace": "ns1", + }, + }) + r1, _ := resFactory.FromMapWithName("config", map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "config-bar", + "namespace": "ns1", + }, + }) + r2, _ := resFactory.FromMapWithName("secret", map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "name": "foo-secret", + "namespace": "ns1", + }, + }) + var resources = []*resource.Resource{r0, r1, r2} expected := resmap.New() for _, r := range resources { err := expected.Append(r) diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index ac01bf0577..fad0b1b4d5 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -73,7 +73,7 @@ func doRemove(t *testing.T, w ResMap, id resid.ResId) { // Make a resource with a predictable name. func makeCm(i int) *resource.Resource { - return rf.FromMap( + r, err := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -81,6 +81,10 @@ func makeCm(i int) *resource.Resource { "name": fmt.Sprintf("cm%03d", i), }, }) + if err != nil { + panic(err) + } + return r } // Maintain the class invariant that no two @@ -229,7 +233,7 @@ metadata: func TestGetMatchingResourcesByCurrentId(t *testing.T) { cmap := resid.NewGvk("", "v1", "ConfigMap") - r1 := rf.FromMap( + r1, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -237,7 +241,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "name": "alice", }, }) - r2 := rf.FromMap( + r2, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -245,7 +249,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "name": "bob", }, }) - r3 := rf.FromMap( + r3, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -254,7 +258,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "namespace": "happy", }, }) - r4 := rf.FromMap( + r4, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -263,7 +267,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "namespace": "happy", }, }) - r5 := rf.FromMap( + r5, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "Deployment", @@ -367,7 +371,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { } func TestGetMatchingResourcesByAnyId(t *testing.T) { - r1 := rf.FromMap( + r1, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -380,7 +384,7 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r2 := rf.FromMap( + r2, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -393,7 +397,7 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r3 := rf.FromMap( + r3, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -407,7 +411,7 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r4 := rf.FromMap( + r4, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -421,7 +425,7 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r5 := rf.FromMap( + r5, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "Deployment", @@ -498,7 +502,7 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { } func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { - r1 := rf.FromMap( + r1, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -506,7 +510,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "name": "alice", }, }) - r2 := rf.FromMap( + r2, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -514,7 +518,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "name": "bob", }, }) - r3 := rf.FromMap( + r3, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -523,7 +527,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "namespace": "happy", }, }) - r4 := rf.FromMap( + r4, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -532,7 +536,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "namespace": "happy", }, }) - r5 := rf.FromMap( + r5, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -542,7 +546,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { }, }) r5.AddNamePrefix("little-") - r6 := rf.FromMap( + r6, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -552,7 +556,7 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { }, }) r6.AddNamePrefix("little-") - r7 := rf.FromMap( + r7, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRoleBinding", @@ -639,7 +643,7 @@ func TestDeepCopy(t *testing.T) { } func TestErrorIfNotEqualSets(t *testing.T) { - r1 := rf.FromMap( + r1, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -647,7 +651,7 @@ func TestErrorIfNotEqualSets(t *testing.T) { "name": "cm1", }, }) - r2 := rf.FromMap( + r2, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -655,7 +659,7 @@ func TestErrorIfNotEqualSets(t *testing.T) { "name": "cm2", }, }) - r3 := rf.FromMap( + r3, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -712,7 +716,7 @@ func TestErrorIfNotEqualSets(t *testing.T) { } func TestErrorIfNotEqualLists(t *testing.T) { - r1 := rf.FromMap( + r1, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -720,7 +724,7 @@ func TestErrorIfNotEqualLists(t *testing.T) { "name": "cm1", }, }) - r2 := rf.FromMap( + r2, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -728,7 +732,7 @@ func TestErrorIfNotEqualLists(t *testing.T) { "name": "cm2", }, }) - r3 := rf.FromMap( + r3, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -780,7 +784,7 @@ func TestErrorIfNotEqualLists(t *testing.T) { } func TestAppendAll(t *testing.T) { - r1 := rf.FromMap( + r1, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -789,7 +793,7 @@ func TestAppendAll(t *testing.T) { }, }) input1 := rmF.FromResource(r1) - r2 := rf.FromMap( + r2, _ := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "StatefulSet", diff --git a/api/resource/factory.go b/api/resource/factory.go index f67fbf3607..fef2f6e499 100644 --- a/api/resource/factory.go +++ b/api/resource/factory.go @@ -41,28 +41,26 @@ func (rf *Factory) Hasher() ifc.KustHasher { } // FromMap returns a new instance of Resource. -func (rf *Factory) FromMap(m map[string]interface{}) *Resource { +func (rf *Factory) FromMap(m map[string]interface{}) (*Resource, error) { res, err := rf.FromMapAndOption(m, nil) if err != nil { - // TODO: return err instead of log. - log.Fatalf("failed to create resource from map: %v", err) + return nil, fmt.Errorf("failed to create resource from map: %w", err) } - return res + return res, nil } // FromMapWithName returns a new instance with the given "original" name. -func (rf *Factory) FromMapWithName(n string, m map[string]interface{}) *Resource { +func (rf *Factory) FromMapWithName(n string, m map[string]interface{}) (*Resource, error) { return rf.FromMapWithNamespaceAndName(resid.DefaultNamespace, n, m) } // FromMapWithNamespaceAndName returns a new instance with the given "original" namespace. -func (rf *Factory) FromMapWithNamespaceAndName(ns string, n string, m map[string]interface{}) *Resource { +func (rf *Factory) FromMapWithNamespaceAndName(ns string, n string, m map[string]interface{}) (*Resource, error) { r, err := rf.FromMapAndOption(m, nil) if err != nil { - // TODO: return err instead of log. - log.Fatalf("failed to create resource from map: %v", err) + return nil, fmt.Errorf("failed to create resource from map: %w", err) } - return r.setPreviousId(ns, n, r.GetKind()) + return r.setPreviousId(ns, n, r.GetKind()), nil } // FromMapAndOption returns a new instance of Resource with given options. diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index 478ed53af0..a79189664e 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -274,7 +274,7 @@ kind: List }, }, } - testDeploymentA := factory.FromMap( + testDeploymentA, _ := factory.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -283,7 +283,7 @@ kind: List }, "spec": testDeploymentSpec, }) - testDeploymentB := factory.FromMap( + testDeploymentB, _ := factory.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -308,6 +308,16 @@ kind: List t.Fatal(err) } + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + + tc, err := createTestConfigMap() + if err != nil { + t.Fatal(err) + } + tests := map[string]struct { input []types.PatchStrategicMerge expectedOut []*Resource @@ -315,7 +325,7 @@ kind: List }{ "happy": { input: []types.PatchStrategicMerge{patchGood1, patchGood2}, - expectedOut: []*Resource{testDeployment, testConfigMap}, + expectedOut: []*Resource{td, tc}, expectedErr: false, }, "badFileName": { @@ -330,7 +340,7 @@ kind: List }, "listOfPatches": { input: []types.PatchStrategicMerge{patchList}, - expectedOut: []*Resource{testDeployment, testConfigMap}, + expectedOut: []*Resource{td, tc}, expectedErr: false, }, "listWithAnchorReference": { diff --git a/api/resource/idset_test.go b/api/resource/idset_test.go index 79cef5a0a4..5b17002538 100644 --- a/api/resource/idset_test.go +++ b/api/resource/idset_test.go @@ -12,21 +12,45 @@ import ( func TestIdSet_Empty(t *testing.T) { s := MakeIdSet([]*Resource{}) + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + tc, err := createTestConfigMap() + if err != nil { + t.Fatal(err) + } assert.Equal(t, 0, s.Size()) - assert.False(t, s.Contains(testDeployment.CurId())) - assert.False(t, s.Contains(testConfigMap.CurId())) + assert.False(t, s.Contains(td.CurId())) + assert.False(t, s.Contains(tc.CurId())) } func TestIdSet_One(t *testing.T) { - s := MakeIdSet([]*Resource{testDeployment}) + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + tc, err := createTestConfigMap() + if err != nil { + t.Fatal(err) + } + s := MakeIdSet([]*Resource{td}) assert.Equal(t, 1, s.Size()) - assert.True(t, s.Contains(testDeployment.CurId())) - assert.False(t, s.Contains(testConfigMap.CurId())) + assert.True(t, s.Contains(td.CurId())) + assert.False(t, s.Contains(tc.CurId())) } func TestIdSet_Two(t *testing.T) { - s := MakeIdSet([]*Resource{testDeployment, testConfigMap}) + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + tc, err := createTestConfigMap() + if err != nil { + t.Fatal(err) + } + s := MakeIdSet([]*Resource{td, tc}) assert.Equal(t, 2, s.Size()) - assert.True(t, s.Contains(testDeployment.CurId())) - assert.True(t, s.Contains(testConfigMap.CurId())) + assert.True(t, s.Contains(td.CurId())) + assert.True(t, s.Contains(tc.CurId())) } diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 46c1bcd179..f85486b726 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -20,27 +20,39 @@ import ( var factory = provider.NewDefaultDepProvider().GetResourceFactory() -var testConfigMap = factory.FromMap( - map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": map[string]interface{}{ - "name": "winnie", - "namespace": "hundred-acre-wood", - }, - }) +func createTestConfigMap() (*Resource, error) { + res, err := factory.FromMap( + map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]interface{}{ + "name": "winnie", + "namespace": "hundred-acre-wood", + }, + }) + if err != nil { + return nil, fmt.Errorf("failed to create test config: %w", err) + } + return res, nil +} //nolint:gosec const configMapAsString = `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie","namespace":"hundred-acre-wood"}}` -var testDeployment = factory.FromMap( - map[string]interface{}{ - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": map[string]interface{}{ - "name": "pooh", - }, - }) +func createTestDeployment() (*Resource, error) { + res, err := factory.FromMap( + map[string]interface{}{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": map[string]interface{}{ + "name": "pooh", + }, + }) + if err != nil { + return nil, fmt.Errorf("failed to create Deployment: %w", err) + } + return res, nil +} const deploymentAsString = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"pooh"}}` @@ -50,7 +62,11 @@ kind: Deployment metadata: name: pooh ` - yaml, err := testDeployment.AsYAML() + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + yaml, err := td.AsYAML() if err != nil { t.Fatal(err) } @@ -60,16 +76,24 @@ metadata: } func TestResourceString(t *testing.T) { + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + tc, err := createTestConfigMap() + if err != nil { + t.Fatal(err) + } tests := []struct { in *Resource s string }{ { - in: testConfigMap, + in: tc, s: configMapAsString, }, { - in: testDeployment, + in: td, s: deploymentAsString, }, } @@ -79,18 +103,26 @@ func TestResourceString(t *testing.T) { } func TestResourceId(t *testing.T) { + td, err := createTestDeployment() + if err != nil { + t.Fatal(err) + } + tc, err := createTestConfigMap() + if err != nil { + t.Fatal(err) + } tests := []struct { in *Resource id resid.ResId }{ { - in: testConfigMap, + in: tc, id: resid.NewResIdWithNamespace( resid.NewGvk("", "v1", "ConfigMap"), "winnie", "hundred-acre-wood"), }, { - in: testDeployment, + in: td, id: resid.NewResId( resid.NewGvk("apps", "v1", "Deployment"), "pooh"), }, @@ -103,7 +135,7 @@ func TestResourceId(t *testing.T) { } func TestDeepCopy(t *testing.T) { - r := factory.FromMap( + r, err := factory.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -111,6 +143,9 @@ func TestDeepCopy(t *testing.T) { "name": "pooh", }, }) + if err != nil { + t.Fatal(err) + } r.AppendRefBy(resid.NewResId(resid.Gvk{Group: "somegroup", Kind: "MyKind"}, "random")) var1 := types.Var{ diff --git a/api/testutils/resmaptest/rmbuilder.go b/api/testutils/resmaptest/rmbuilder.go index 1eea23b191..12cdf850f9 100644 --- a/api/testutils/resmaptest/rmbuilder.go +++ b/api/testutils/resmaptest/rmbuilder.go @@ -41,7 +41,11 @@ func NewSeededRmBuilderDefault(t *testing.T, m resmap.ResMap) *rmBuilder { } func (rm *rmBuilder) Add(m map[string]interface{}) *rmBuilder { - return rm.AddR(rm.rf.FromMap(m)) + r, err := rm.rf.FromMap(m) + if err != nil { + rm.t.Fatalf("test setup failure: %v", err) + } + return rm.AddR(r) } func (rm *rmBuilder) AddR(r *resource.Resource) *rmBuilder { @@ -53,7 +57,11 @@ func (rm *rmBuilder) AddR(r *resource.Resource) *rmBuilder { } func (rm *rmBuilder) AddWithName(n string, m map[string]interface{}) *rmBuilder { - err := rm.m.Append(rm.rf.FromMapWithNamespaceAndName(resid.DefaultNamespace, n, m)) + r, err := rm.rf.FromMapWithNamespaceAndName(resid.DefaultNamespace, n, m) + if err != nil { + rm.t.Fatalf("test setup failure: %v", err) + } + err = rm.m.Append(r) if err != nil { rm.t.Fatalf("test setup failure: %v", err) } @@ -61,7 +69,11 @@ func (rm *rmBuilder) AddWithName(n string, m map[string]interface{}) *rmBuilder } func (rm *rmBuilder) AddWithNsAndName(ns string, n string, m map[string]interface{}) *rmBuilder { - err := rm.m.Append(rm.rf.FromMapWithNamespaceAndName(ns, n, m)) + r, err := rm.rf.FromMapWithNamespaceAndName(ns, n, m) + if err != nil { + rm.t.Fatalf("test setup failure: %v", err) + } + err = rm.m.Append(r) if err != nil { rm.t.Fatalf("test setup failure: %v", err) } @@ -69,7 +81,7 @@ func (rm *rmBuilder) AddWithNsAndName(ns string, n string, m map[string]interfac } func (rm *rmBuilder) ReplaceResource(m map[string]interface{}) *rmBuilder { - r := rm.rf.FromMap(m) + r, _ := rm.rf.FromMap(m) _, err := rm.m.Replace(r) if err != nil { rm.t.Fatalf("test setup failure: %v", err) From 0575a3aa8ac1fa948653b0d7d5fa22ca832fad6b Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 6 Apr 2024 22:16:55 +0900 Subject: [PATCH 2/3] chore: add meaningful message to error output --- .../namereferencetransformer_test.go | 16 ++- .../accumulator/resaccumulator_test.go | 23 +++- .../plugins/execplugin/execplugin_test.go | 2 +- api/internal/target/kusttarget_test.go | 38 +++++- api/resmap/reswrangler_test.go | 125 ++++++++++++++---- api/resource/factory_test.go | 16 ++- api/resource/idset_test.go | 12 +- api/resource/resource_test.go | 8 +- 8 files changed, 182 insertions(+), 58 deletions(-) diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index d54310c4eb..84c3242f8d 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -590,7 +590,7 @@ func TestNameReferenceUnhappyRun(t *testing.T) { func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { rf := provider.NewDefaultDepProvider().GetResourceFactory() - v1, _ := rf.FromMapWithName( + v1, err := rf.FromMapWithName( "volume1", map[string]interface{}{ "apiVersion": "v1", @@ -599,7 +599,10 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "name": "someprefix-volume1", }, }) - c1, _ := rf.FromMapWithName( + if err != nil { + t.Fatalf("failed to get new instance with given name: %v", err) + } + c1, err := rf.FromMapWithName( "claim1", map[string]interface{}{ "apiVersion": "v1", @@ -612,9 +615,11 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "volumeName": "volume1", }, }) - + if err != nil { + t.Fatalf("failed to get new instance with given name: %v", err) + } v2 := v1.DeepCopy() - c2, _ := rf.FromMapWithName( + c2, err := rf.FromMapWithName( "claim1", map[string]interface{}{ "apiVersion": "v1", @@ -627,6 +632,9 @@ func TestNameReferencePersistentVolumeHappyRun(t *testing.T) { "volumeName": "someprefix-volume1", }, }) + if err != nil { + t.Fatalf("failed to get new instance with given name: %v", err) + } m1 := resmaptest_test.NewRmBuilder(t, rf).AddR(v1).AddR(c1).ResMap() diff --git a/api/internal/accumulator/resaccumulator_test.go b/api/internal/accumulator/resaccumulator_test.go index 5c3263c51d..6172c3f29c 100644 --- a/api/internal/accumulator/resaccumulator_test.go +++ b/api/internal/accumulator/resaccumulator_test.go @@ -64,7 +64,7 @@ func makeResAccumulator(t *testing.T) *ResAccumulator { "name": "backendTwo", }}).ResMap()) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf("failed to append resources: %v", err) } return ra } @@ -154,15 +154,15 @@ func TestResolveVarsVarNeedsDisambiguation(t *testing.T) { }, }) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf("failed to get instance of resources: %v", err) } err = rm0.Append(r) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf("failed to append a resource to ResMap: %v", err) } err = ra.AppendAll(rm0) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf("failed to append a resource to ResAccumulator: %v", err) } err = ra.MergeVars([]types.Var{ @@ -231,7 +231,10 @@ func TestResolveVarConflicts(t *testing.T) { // create accumulators holding apparently conflicting vars that are not // actually in conflict because they point to the same concrete value. rm0 := resmap.New() - r0, _ := rf.FromMap(fooAws) + r0, err0 := rf.FromMap(fooAws) + if err0 != nil { + t.Fatalf("failed to get instance of resources: %v", err0) + } err := rm0.Append(r0) require.NoError(t, err) ac0 := MakeEmptyAccumulator() @@ -241,7 +244,10 @@ func TestResolveVarConflicts(t *testing.T) { require.NoError(t, err) rm1 := resmap.New() - r1, _ := rf.FromMap(barAws) + r1, err1 := rf.FromMap(barAws) + if err1 != nil { + t.Fatalf("failed to get instance of resources: %v", err1) + } err = rm1.Append(r1) require.NoError(t, err) ac1 := MakeEmptyAccumulator() @@ -261,7 +267,10 @@ func TestResolveVarConflicts(t *testing.T) { // two above (because it contains a variable whose name is used in the other // accumulators AND whose concrete values are different). rm2 := resmap.New() - r2, _ := rf.FromMap(barGcp) + r2, err2 := rf.FromMap(barGcp) + if err2 != nil { + t.Fatalf("failed to get instance of resources: %v", err2) + } err = rm2.Append(r2) require.NoError(t, err) ac2 := MakeEmptyAccumulator() diff --git a/api/internal/plugins/execplugin/execplugin_test.go b/api/internal/plugins/execplugin/execplugin_test.go index c21d20acf7..911ab8e7b4 100644 --- a/api/internal/plugins/execplugin/execplugin_test.go +++ b/api/internal/plugins/execplugin/execplugin_test.go @@ -52,7 +52,7 @@ s/$BAR/bar baz/g "argsFromFile": "sed-input.txt", }) if err != nil { - t.Fatalf("unexpected err: %v", err) + t.Fatalf("failed to writes the data to a file: %v", err) } pluginConfig.RemoveBuildAnnotations() diff --git a/api/internal/target/kusttarget_test.go b/api/internal/target/kusttarget_test.go index 5dfb086693..03f082bb16 100644 --- a/api/internal/target/kusttarget_test.go +++ b/api/internal/target/kusttarget_test.go @@ -189,8 +189,9 @@ metadata: pvd := provider.NewDefaultDepProvider() resFactory := pvd.GetResourceFactory() + name0 := "dply1" - r0, _ := resFactory.FromMapWithName("dply1", map[string]interface{}{ + r0, err := resFactory.FromMapWithName(name0, map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": map[string]interface{}{ @@ -222,7 +223,11 @@ metadata: }, }, }) - r1, _ := resFactory.FromMapWithName("ns1", map[string]interface{}{ + if err != nil { + t.Fatalf("failed to get instance with given name %v: %v", name0, err) + } + name1 := "ns1" + r1, err := resFactory.FromMapWithName(name1, map[string]interface{}{ "apiVersion": "v1", "kind": "Namespace", "metadata": map[string]interface{}{ @@ -235,6 +240,9 @@ metadata: }, }, }) + if err != nil { + t.Fatalf("failed to get instance with given name %v: %v", name1, err) + } r2, _ := resFactory.FromMapWithName("literalConfigMap", map[string]interface{}{ @@ -256,7 +264,8 @@ metadata: }, }) - r3, _ := resFactory.FromMapWithName("secret", + name2 := "secret" + r3, err := resFactory.FromMapWithName(name2, map[string]interface{}{ "apiVersion": "v1", "kind": "Secret", @@ -276,13 +285,16 @@ metadata: "DB_PASSWORD": base64.StdEncoding.EncodeToString([]byte("somepw")), }, }) + if err != nil { + t.Fatalf("failed to get instance with given name %v: %v", name2, err) + } resources := []*resource.Resource{r0, r1, r2, r3} expected := resmap.New() for _, r := range resources { if err := expected.Append(r); err != nil { - t.Fatalf("unexpected error %v", err) + t.Fatalf("failed to append resource: %v", err) } } expected.RemoveBuildAnnotations() @@ -354,7 +366,8 @@ metadata: pvd := provider.NewDefaultDepProvider() resFactory := pvd.GetResourceFactory() - r0, _ := resFactory.FromMapWithName("deployment1", map[string]interface{}{ + name0 := "deployment1" + r0, err0 := resFactory.FromMapWithName(name0, map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": map[string]interface{}{ @@ -362,7 +375,11 @@ metadata: "namespace": "ns1", }, }) - r1, _ := resFactory.FromMapWithName("config", map[string]interface{}{ + if err0 != nil { + t.Fatalf("failed to get instance with given name %v: %v", name0, err0) + } + name1 := "config" + r1, err1 := resFactory.FromMapWithName(name1, map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", "metadata": map[string]interface{}{ @@ -370,7 +387,11 @@ metadata: "namespace": "ns1", }, }) - r2, _ := resFactory.FromMapWithName("secret", map[string]interface{}{ + if err1 != nil { + t.Fatalf("failed to get instance with given name %v: %v", name1, err1) + } + name2 := "secret" + r2, err2 := resFactory.FromMapWithName(name2, map[string]interface{}{ "apiVersion": "v1", "kind": "Secret", "metadata": map[string]interface{}{ @@ -378,6 +399,9 @@ metadata: "namespace": "ns1", }, }) + if err2 != nil { + t.Fatalf("failed to get instance with given name %v: %v", name2, err2) + } var resources = []*resource.Resource{r0, r1, r2} expected := resmap.New() for _, r := range resources { diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go index fad0b1b4d5..73c7d71425 100644 --- a/api/resmap/reswrangler_test.go +++ b/api/resmap/reswrangler_test.go @@ -233,7 +233,7 @@ metadata: func TestGetMatchingResourcesByCurrentId(t *testing.T) { cmap := resid.NewGvk("", "v1", "ConfigMap") - r1, _ := rf.FromMap( + r1, err1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -241,7 +241,10 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "name": "alice", }, }) - r2, _ := rf.FromMap( + if err1 != nil { + t.Fatalf("failed to get new instance: %v", err1) + } + r2, err2 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -249,7 +252,10 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "name": "bob", }, }) - r3, _ := rf.FromMap( + if err2 != nil { + t.Fatalf("failed to get new instance: %v", err2) + } + r3, err3 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -258,7 +264,10 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "namespace": "happy", }, }) - r4, _ := rf.FromMap( + if err3 != nil { + t.Fatalf("failed to get new instance: %v", err3) + } + r4, err4 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -267,7 +276,10 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "namespace": "happy", }, }) - r5, _ := rf.FromMap( + if err4 != nil { + t.Fatalf("failed to get new instance: %v", err4) + } + r5, err5 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "Deployment", @@ -276,6 +288,9 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { "namespace": "happy", }, }) + if err5 != nil { + t.Fatalf("failed to get new instance: %v", err5) + } m := resmaptest_test.NewRmBuilder(t, rf). AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).ResMap() @@ -371,7 +386,7 @@ func TestGetMatchingResourcesByCurrentId(t *testing.T) { } func TestGetMatchingResourcesByAnyId(t *testing.T) { - r1, _ := rf.FromMap( + r1, err1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -384,7 +399,10 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r2, _ := rf.FromMap( + if err1 != nil { + t.Fatalf("failed to get new instance: %v", err1) + } + r2, err2 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -397,7 +415,10 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r3, _ := rf.FromMap( + if err2 != nil { + t.Fatalf("failed to get new instance: %v", err2) + } + r3, err3 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -411,7 +432,10 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r4, _ := rf.FromMap( + if err3 != nil { + t.Fatalf("failed to get new instance: %v", err3) + } + r4, err4 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -425,7 +449,10 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { }, }, }) - r5, _ := rf.FromMap( + if err4 != nil { + t.Fatalf("failed to get new instance: %v", err4) + } + r5, err5 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "Deployment", @@ -434,6 +461,9 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { "namespace": "happy", }, }) + if err5 != nil { + t.Fatalf("failed to get new instance: %v", err5) + } m := resmaptest_test.NewRmBuilder(t, rf). AddR(r1).AddR(r2).AddR(r3).AddR(r4).AddR(r5).ResMap() @@ -502,7 +532,7 @@ func TestGetMatchingResourcesByAnyId(t *testing.T) { } func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { - r1, _ := rf.FromMap( + r1, err1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -510,7 +540,10 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "name": "alice", }, }) - r2, _ := rf.FromMap( + if err1 != nil { + t.Fatalf("failed to get new instance: %v", err1) + } + r2, err2 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -518,7 +551,10 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "name": "bob", }, }) - r3, _ := rf.FromMap( + if err2 != nil { + t.Fatalf("failed to get new instance: %v", err2) + } + r3, err3 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -527,7 +563,10 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "namespace": "happy", }, }) - r4, _ := rf.FromMap( + if err3 != nil { + t.Fatalf("failed to get new instance: %v", err3) + } + r4, err4 := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -536,7 +575,10 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "namespace": "happy", }, }) - r5, _ := rf.FromMap( + if err4 != nil { + t.Fatalf("failed to get new instance: %v", err4) + } + r5, err5 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -545,8 +587,11 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "namespace": "happy", }, }) + if err5 != nil { + t.Fatalf("failed to get new instance: %v", err5) + } r5.AddNamePrefix("little-") - r6, _ := rf.FromMap( + r6, err6 := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -555,8 +600,11 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "namespace": "happy", }, }) + if err6 != nil { + t.Fatalf("failed to get new instance: %v", err6) + } r6.AddNamePrefix("little-") - r7, _ := rf.FromMap( + r7, err7 := rf.FromMap( map[string]interface{}{ "apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRoleBinding", @@ -564,6 +612,9 @@ func TestSubsetThatCouldBeReferencedByResource(t *testing.T) { "name": "meh", }, }) + if err7 != nil { + t.Fatalf("failed to get new instance: %v", err7) + } tests := map[string]struct { filter *resource.Resource @@ -643,7 +694,7 @@ func TestDeepCopy(t *testing.T) { } func TestErrorIfNotEqualSets(t *testing.T) { - r1, _ := rf.FromMap( + r1, err1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -651,7 +702,10 @@ func TestErrorIfNotEqualSets(t *testing.T) { "name": "cm1", }, }) - r2, _ := rf.FromMap( + if err1 != nil { + t.Fatalf("failed to get new instance: %v", err1) + } + r2, err2 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -659,7 +713,10 @@ func TestErrorIfNotEqualSets(t *testing.T) { "name": "cm2", }, }) - r3, _ := rf.FromMap( + if err2 != nil { + t.Fatalf("failed to get new instance: %v", err2) + } + r3, err3 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -668,6 +725,9 @@ func TestErrorIfNotEqualSets(t *testing.T) { "namespace": "system", }, }) + if err3 != nil { + t.Fatalf("failed to get new instance: %v", err3) + } m1 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).AddR(r2).AddR(r3).ResMap() if err := m1.ErrorIfNotEqualSets(m1); err != nil { @@ -716,7 +776,7 @@ func TestErrorIfNotEqualSets(t *testing.T) { } func TestErrorIfNotEqualLists(t *testing.T) { - r1, _ := rf.FromMap( + r1, err1 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -724,7 +784,10 @@ func TestErrorIfNotEqualLists(t *testing.T) { "name": "cm1", }, }) - r2, _ := rf.FromMap( + if err1 != nil { + t.Fatalf("failed to get new instance: %v", err1) + } + r2, err2 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -732,7 +795,10 @@ func TestErrorIfNotEqualLists(t *testing.T) { "name": "cm2", }, }) - r3, _ := rf.FromMap( + if err2 != nil { + t.Fatalf("failed to get new instance: %v", err2) + } + r3, err3 := rf.FromMap( map[string]interface{}{ "apiVersion": "v1", "kind": "ConfigMap", @@ -741,6 +807,9 @@ func TestErrorIfNotEqualLists(t *testing.T) { "namespace": "system", }, }) + if err3 != nil { + t.Fatalf("failed to get new instance: %v", err3) + } m1 := resmaptest_test.NewRmBuilder(t, rf).AddR(r1).AddR(r2).AddR(r3).ResMap() if err := m1.ErrorIfNotEqualLists(m1); err != nil { @@ -784,7 +853,7 @@ func TestErrorIfNotEqualLists(t *testing.T) { } func TestAppendAll(t *testing.T) { - r1, _ := rf.FromMap( + r1, err1 := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", @@ -792,8 +861,11 @@ func TestAppendAll(t *testing.T) { "name": "foo-deploy1", }, }) + if err1 != nil { + t.Fatalf("failed to get new instance: %v", err1) + } input1 := rmF.FromResource(r1) - r2, _ := rf.FromMap( + r2, err2 := rf.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "StatefulSet", @@ -801,6 +873,9 @@ func TestAppendAll(t *testing.T) { "name": "bar-stateful", }, }) + if err2 != nil { + t.Fatalf("failed to get new instance: %v", err2) + } input2 := rmF.FromResource(r2) expected := New() diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index a79189664e..9edbb7bba2 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -274,24 +274,32 @@ kind: List }, }, } - testDeploymentA, _ := factory.FromMap( + deploymentA := "deployment-a" + testDeploymentA, errA := factory.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": map[string]interface{}{ - "name": "deployment-a", + "name": deploymentA, }, "spec": testDeploymentSpec, }) - testDeploymentB, _ := factory.FromMap( + if errA != nil { + t.Fatalf("Failed to create new instance with %v: %v", deploymentA, errA) + } + deploymentB := "deployment-b" + testDeploymentB, errB := factory.FromMap( map[string]interface{}{ "apiVersion": "apps/v1", "kind": "Deployment", "metadata": map[string]interface{}{ - "name": "deployment-b", + "name": deploymentB, }, "spec": testDeploymentSpec, }) + if errB != nil { + t.Fatalf("Failed to create new instance with %v: %v", deploymentB, errB) + } fSys := filesys.MakeFsInMemory() fSys.WriteFile(string(patchGood1), []byte(patch1)) diff --git a/api/resource/idset_test.go b/api/resource/idset_test.go index 5b17002538..ca8711046b 100644 --- a/api/resource/idset_test.go +++ b/api/resource/idset_test.go @@ -14,11 +14,11 @@ func TestIdSet_Empty(t *testing.T) { s := MakeIdSet([]*Resource{}) td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("Failed to create test deployment: %v", err) } tc, err := createTestConfigMap() if err != nil { - t.Fatal(err) + t.Fatalf("Failed to create test config: %v", err) } assert.Equal(t, 0, s.Size()) assert.False(t, s.Contains(td.CurId())) @@ -28,11 +28,11 @@ func TestIdSet_Empty(t *testing.T) { func TestIdSet_One(t *testing.T) { td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test deployment: %v", err) } tc, err := createTestConfigMap() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test config: %v", err) } s := MakeIdSet([]*Resource{td}) assert.Equal(t, 1, s.Size()) @@ -43,11 +43,11 @@ func TestIdSet_One(t *testing.T) { func TestIdSet_Two(t *testing.T) { td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test Deployment: %v", err) } tc, err := createTestConfigMap() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test Config: %v", err) } s := MakeIdSet([]*Resource{td, tc}) assert.Equal(t, 2, s.Size()) diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index f85486b726..035a7364f1 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -64,11 +64,11 @@ metadata: ` td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test deployment: %s", err) } yaml, err := td.AsYAML() if err != nil { - t.Fatal(err) + t.Fatalf("failed to get yaml: %s", err) } if string(yaml) != expected { t.Fatalf("--- expected\n%s\n--- got\n%s\n", expected, string(yaml)) @@ -105,11 +105,11 @@ func TestResourceString(t *testing.T) { func TestResourceId(t *testing.T) { td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test deployment: %v", err) } tc, err := createTestConfigMap() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test config: %v", err) } tests := []struct { in *Resource From 31abd0bd8172c62a615f9ae438cf6d622608996c Mon Sep 17 00:00:00 2001 From: chansuke Date: Thu, 18 Apr 2024 19:05:21 +0900 Subject: [PATCH 3/3] chore: add meaningful message to fatal function --- api/resource/factory_test.go | 8 ++++---- api/resource/resource_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index 9edbb7bba2..8695ab0364 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -285,7 +285,7 @@ kind: List "spec": testDeploymentSpec, }) if errA != nil { - t.Fatalf("Failed to create new instance with %v: %v", deploymentA, errA) + t.Fatalf("failed to create new instance with %v: %v", deploymentA, errA) } deploymentB := "deployment-b" testDeploymentB, errB := factory.FromMap( @@ -298,7 +298,7 @@ kind: List "spec": testDeploymentSpec, }) if errB != nil { - t.Fatalf("Failed to create new instance with %v: %v", deploymentB, errB) + t.Fatalf("failed to create new instance with %v: %v", deploymentB, errB) } fSys := filesys.MakeFsInMemory() @@ -318,12 +318,12 @@ kind: List td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test deployment: %v", err) } tc, err := createTestConfigMap() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test config: %v", err) } tests := map[string]struct { diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 035a7364f1..858806ad2b 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -78,11 +78,11 @@ metadata: func TestResourceString(t *testing.T) { td, err := createTestDeployment() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test deployment: %v", err) } tc, err := createTestConfigMap() if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test config: %v", err) } tests := []struct { in *Resource @@ -144,7 +144,7 @@ func TestDeepCopy(t *testing.T) { }, }) if err != nil { - t.Fatal(err) + t.Fatalf("failed to create test config: %v", err) } r.AppendRefBy(resid.NewResId(resid.Gvk{Group: "somegroup", Kind: "MyKind"}, "random"))