From 7a73fc3a08cdd66535bfa4bb1922e6a6be061e49 Mon Sep 17 00:00:00 2001 From: "xian-jie.shen" <327411586@qq.com> Date: Fri, 15 Jul 2022 12:01:51 +0800 Subject: [PATCH] perf: improve the FillMapWithStrAndError performance and split too long test case Signed-off-by: xian-jie.shen <327411586@qq.com> --- pkg/util/helm/helm_operation_test.go | 193 +++++++++++++++++++++++++++ pkg/util/helm/helm_test.go | 184 ------------------------- pkg/util/mapz/map.go | 2 +- pkg/util/mapz/map_test.go | 28 +++- 4 files changed, 215 insertions(+), 192 deletions(-) create mode 100644 pkg/util/helm/helm_operation_test.go diff --git a/pkg/util/helm/helm_operation_test.go b/pkg/util/helm/helm_operation_test.go new file mode 100644 index 000000000..eb3819bef --- /dev/null +++ b/pkg/util/helm/helm_operation_test.go @@ -0,0 +1,193 @@ +package helm + +import ( + "testing" + "time" + + helmclient "github.com/mittwald/go-helm-client" + "helm.sh/helm/v3/pkg/repo" +) + +func TestInstallOrUpgradeChart(t *testing.T) { + atomic := true + if !helmParam.Chart.Wait { + atomic = false + } + tmout, err := time.ParseDuration(helmParam.Chart.Timeout) + if err != nil { + t.Log(err) + } + chartSpec := &helmclient.ChartSpec{ + ReleaseName: helmParam.Chart.ReleaseName, + ChartName: helmParam.Chart.ChartName, + Namespace: helmParam.Chart.Namespace, + ValuesYaml: helmParam.Chart.ValuesYaml, + Version: helmParam.Chart.Version, + CreateNamespace: false, + DisableHooks: false, + Replace: true, + Wait: helmParam.Chart.Wait, + DependencyUpdate: false, + Timeout: tmout, + GenerateName: false, + NameTemplate: "", + Atomic: atomic, + SkipCRDs: false, + UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + SubNotes: false, + Force: false, + ResetValues: false, + ReuseValues: false, + Recreate: false, + MaxHistory: 0, + CleanupOnFail: false, + DryRun: false, + } + + // ctrl := gomock.NewController(t) + // defer ctrl.Finish() + + // mockClient := mockhelmclient.NewMockClient(ctrl) + // if mockClient == nil { + // t.Fail() + // } + h, err := NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient{})) + + if err != nil { + t.Errorf("error: %v\n", err) + } + // mockClient.EXPECT().InstallOrUpgradeChart(context.TODO(), chartSpec).Return(&mockedRelease, nil) + + err = h.InstallOrUpgradeChart() + if err != nil { + t.Error(err) + } +} + +func TestAddOrUpdateChartRepo(t *testing.T) { + entry := &repo.Entry{ + Name: helmParam.Repo.Name, + URL: helmParam.Repo.URL, + Username: "", + Password: "", + CertFile: "", + KeyFile: "", + CAFile: "", + InsecureSkipTLSverify: false, + PassCredentialsAll: false, + } + atomic := true + if !helmParam.Chart.Wait { + atomic = false + } + tmout, err := time.ParseDuration(helmParam.Chart.Timeout) + if err != nil { + t.Log(err) + } + chartSpec := &helmclient.ChartSpec{ + ReleaseName: helmParam.Chart.ReleaseName, + ChartName: helmParam.Chart.ChartName, + Namespace: helmParam.Chart.Namespace, + ValuesYaml: helmParam.Chart.ValuesYaml, + Version: helmParam.Chart.Version, + CreateNamespace: false, + DisableHooks: false, + Replace: true, + Wait: helmParam.Chart.Wait, + DependencyUpdate: false, + Timeout: tmout, + GenerateName: false, + NameTemplate: "", + Atomic: atomic, + SkipCRDs: false, + UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + SubNotes: false, + Force: false, + ResetValues: false, + ReuseValues: false, + Recreate: false, + MaxHistory: 0, + CleanupOnFail: false, + DryRun: false, + } + + h, err := NewHelm(helmParam, WithEntry(entry), WithChartSpec(chartSpec), WithClient(&DefaultMockClient{})) + if err != nil { + t.Errorf("error: %v\n", err) + } + + err = h.AddOrUpdateChartRepo(*entry) + if err != nil { + t.Error(err) + } +} + +func TestHelm_UninstallHelmChartRelease(t *testing.T) { + atomic := true + if !helmParam.Chart.Wait { + atomic = false + } + tmout, err := time.ParseDuration(helmParam.Chart.Timeout) + if err != nil { + t.Log(err) + } + chartSpec := &helmclient.ChartSpec{ + ReleaseName: helmParam.Chart.ReleaseName, + ChartName: helmParam.Chart.ChartName, + Namespace: helmParam.Chart.Namespace, + ValuesYaml: helmParam.Chart.ValuesYaml, + Version: helmParam.Chart.Version, + CreateNamespace: false, + DisableHooks: false, + Replace: true, + Wait: helmParam.Chart.Wait, + DependencyUpdate: false, + Timeout: tmout, + GenerateName: false, + NameTemplate: "", + Atomic: atomic, + SkipCRDs: false, + UpgradeCRDs: helmParam.Chart.UpgradeCRDs, + SubNotes: false, + Force: false, + ResetValues: false, + ReuseValues: false, + Recreate: false, + MaxHistory: 0, + CleanupOnFail: false, + DryRun: false, + } + // base + h, err := NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient3{})) + if err != nil { + t.Errorf("error: %v\n", err) + } + + err = h.UninstallHelmChartRelease() + if err != nil { + t.Error(err) + } + + // mock error not found + h, err = NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient{})) + if err != nil { + t.Errorf("error: %v\n", err) + } + err = h.UninstallHelmChartRelease() + if err != nil { + t.Error(err) + } + + // mock error + h, err = NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient2{})) + if err != nil { + t.Errorf("error: %v\n", err) + } + err = h.UninstallHelmChartRelease() + if err == nil { + t.Error("error not found") + } + if err != NormalError { + t.Errorf("got: %+v\n, want %+v\n", err, NormalError) + } +} diff --git a/pkg/util/helm/helm_test.go b/pkg/util/helm/helm_test.go index 54ca6e476..1d7076e22 100644 --- a/pkg/util/helm/helm_test.go +++ b/pkg/util/helm/helm_test.go @@ -151,187 +151,3 @@ func TestNewHelmWithOption(t *testing.T) { t.Errorf("NewHelm() = \n%+v\n, want \n%+v\n", got, want) } } - -func TestInstallOrUpgradeChart(t *testing.T) { - atomic := true - if !helmParam.Chart.Wait { - atomic = false - } - tmout, err := time.ParseDuration(helmParam.Chart.Timeout) - if err != nil { - t.Log(err) - } - chartSpec := &helmclient.ChartSpec{ - ReleaseName: helmParam.Chart.ReleaseName, - ChartName: helmParam.Chart.ChartName, - Namespace: helmParam.Chart.Namespace, - ValuesYaml: helmParam.Chart.ValuesYaml, - Version: helmParam.Chart.Version, - CreateNamespace: false, - DisableHooks: false, - Replace: true, - Wait: helmParam.Chart.Wait, - DependencyUpdate: false, - Timeout: tmout, - GenerateName: false, - NameTemplate: "", - Atomic: atomic, - SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, - SubNotes: false, - Force: false, - ResetValues: false, - ReuseValues: false, - Recreate: false, - MaxHistory: 0, - CleanupOnFail: false, - DryRun: false, - } - - // ctrl := gomock.NewController(t) - // defer ctrl.Finish() - - // mockClient := mockhelmclient.NewMockClient(ctrl) - // if mockClient == nil { - // t.Fail() - // } - h, err := NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient{})) - - if err != nil { - t.Errorf("error: %v\n", err) - } - // mockClient.EXPECT().InstallOrUpgradeChart(context.TODO(), chartSpec).Return(&mockedRelease, nil) - - err = h.InstallOrUpgradeChart() - if err != nil { - t.Error(err) - } -} - -func TestAddOrUpdateChartRepo(t *testing.T) { - entry := &repo.Entry{ - Name: helmParam.Repo.Name, - URL: helmParam.Repo.URL, - Username: "", - Password: "", - CertFile: "", - KeyFile: "", - CAFile: "", - InsecureSkipTLSverify: false, - PassCredentialsAll: false, - } - atomic := true - if !helmParam.Chart.Wait { - atomic = false - } - tmout, err := time.ParseDuration(helmParam.Chart.Timeout) - if err != nil { - t.Log(err) - } - chartSpec := &helmclient.ChartSpec{ - ReleaseName: helmParam.Chart.ReleaseName, - ChartName: helmParam.Chart.ChartName, - Namespace: helmParam.Chart.Namespace, - ValuesYaml: helmParam.Chart.ValuesYaml, - Version: helmParam.Chart.Version, - CreateNamespace: false, - DisableHooks: false, - Replace: true, - Wait: helmParam.Chart.Wait, - DependencyUpdate: false, - Timeout: tmout, - GenerateName: false, - NameTemplate: "", - Atomic: atomic, - SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, - SubNotes: false, - Force: false, - ResetValues: false, - ReuseValues: false, - Recreate: false, - MaxHistory: 0, - CleanupOnFail: false, - DryRun: false, - } - - h, err := NewHelm(helmParam, WithEntry(entry), WithChartSpec(chartSpec), WithClient(&DefaultMockClient{})) - if err != nil { - t.Errorf("error: %v\n", err) - } - - err = h.AddOrUpdateChartRepo(*entry) - if err != nil { - t.Error(err) - } -} - -func TestHelm_UninstallHelmChartRelease(t *testing.T) { - atomic := true - if !helmParam.Chart.Wait { - atomic = false - } - tmout, err := time.ParseDuration(helmParam.Chart.Timeout) - if err != nil { - t.Log(err) - } - chartSpec := &helmclient.ChartSpec{ - ReleaseName: helmParam.Chart.ReleaseName, - ChartName: helmParam.Chart.ChartName, - Namespace: helmParam.Chart.Namespace, - ValuesYaml: helmParam.Chart.ValuesYaml, - Version: helmParam.Chart.Version, - CreateNamespace: false, - DisableHooks: false, - Replace: true, - Wait: helmParam.Chart.Wait, - DependencyUpdate: false, - Timeout: tmout, - GenerateName: false, - NameTemplate: "", - Atomic: atomic, - SkipCRDs: false, - UpgradeCRDs: helmParam.Chart.UpgradeCRDs, - SubNotes: false, - Force: false, - ResetValues: false, - ReuseValues: false, - Recreate: false, - MaxHistory: 0, - CleanupOnFail: false, - DryRun: false, - } - // base - h, err := NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient3{})) - if err != nil { - t.Errorf("error: %v\n", err) - } - - err = h.UninstallHelmChartRelease() - if err != nil { - t.Error(err) - } - - // mock error not found - h, err = NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient{})) - if err != nil { - t.Errorf("error: %v\n", err) - } - err = h.UninstallHelmChartRelease() - if err != nil { - t.Error(err) - } - - // mock error - h, err = NewHelm(helmParam, WithChartSpec(chartSpec), WithClient(&DefaultMockClient2{})) - if err != nil { - t.Errorf("error: %v\n", err) - } - err = h.UninstallHelmChartRelease() - if err == nil { - t.Error("error not found") - } - if err != NormalError { - t.Errorf("got: %+v\n, want %+v\n", err, NormalError) - } -} diff --git a/pkg/util/mapz/map.go b/pkg/util/mapz/map.go index 79d34f61c..d044215bb 100644 --- a/pkg/util/mapz/map.go +++ b/pkg/util/mapz/map.go @@ -1,7 +1,7 @@ package mapz func FillMapWithStrAndError(keys []string, value error) map[string]error { - retMap := make(map[string]error) + retMap := make(map[string]error, len(keys)) if len(keys) == 0 { return retMap } diff --git a/pkg/util/mapz/map_test.go b/pkg/util/mapz/map_test.go index 0359ab57d..0d9cfc99f 100644 --- a/pkg/util/mapz/map_test.go +++ b/pkg/util/mapz/map_test.go @@ -2,6 +2,8 @@ package mapz import ( "fmt" + "strconv" + "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -12,15 +14,13 @@ var _ = Describe("Mapz", func() { "key1", "key2", } value := fmt.Errorf("error") + expectMap := map[string]error{ + "key1": value, + "key2": value, + } retMap1 := FillMapWithStrAndError(keys, value) It("should be a map with 2 items", func() { - Expect(len(retMap1)).To(Equal(2)) - v1, ok := retMap1["key1"] - Expect(ok).To(Equal(true)) - Expect(v1).To(Equal(value)) - v2, ok := retMap1["key2"] - Expect(ok).To(Equal(true)) - Expect(v2).To(Equal(value)) + Expect(retMap1).Should(Equal(expectMap)) }) retMap2 := FillMapWithStrAndError(nil, value) @@ -28,3 +28,17 @@ var _ = Describe("Mapz", func() { Expect(len(retMap2)).To(Equal(0)) }) }) + +func BenchmarkFillMapWithStrAndError(b *testing.B) { + keys_length := 100 + keys := make([]string, 0, keys_length) + for i := 0; i < keys_length; i++ { + keys = append(keys, "key"+strconv.Itoa(i)) + } + value := fmt.Errorf("error") + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = FillMapWithStrAndError(keys, value) + } + b.StopTimer() +}