From 5c8d82aed08855989fa33b68b62351da454c5c5c Mon Sep 17 00:00:00 2001 From: Oleg Atamanenko Date: Sat, 9 Jun 2018 23:18:40 -0400 Subject: [PATCH] Fixed gocyclo warnings and added pre-commit hook --- .travis.yml | 1 + bin/pre-commit.sh | 6 +++ pkg/commands/build_test.go | 88 +++++++++++++++++----------------- pkg/commands/diff_test.go | 96 ++++++++++++++++++++------------------ 4 files changed, 103 insertions(+), 88 deletions(-) diff --git a/.travis.yml b/.travis.yml index d7d0dfcef..0e45d1d0f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ before_install: - go get -u golang.org/x/tools/cmd/goimports - go get -u github.com/onsi/ginkgo/ginkgo - go get -u github.com/monopole/mdrip + - go get -u github.com/fzipp/gocyclo # Install must be set to prevent default `go get` to run. # The dependencies have already been vendored by `dep` so diff --git a/bin/pre-commit.sh b/bin/pre-commit.sh index 11b0cf91f..b89d4b757 100755 --- a/bin/pre-commit.sh +++ b/bin/pre-commit.sh @@ -31,6 +31,11 @@ function testGoFmt { diff <(echo -n) <(go_dirs | xargs -0 gofmt -s -d -l) } + +function testGoCyclo { + diff <(echo -n) <(go_dirs | xargs -0 gocyclo -over 15) +} + function testGoImports { diff -u <(echo -n) <(go_dirs | xargs -0 goimports -l) } @@ -50,6 +55,7 @@ function testExamples { runTest testGoFmt runTest testGoImports runTest testGoVet +runTest testGoCyclo runTest testGoTest runTest testExamples diff --git a/pkg/commands/build_test.go b/pkg/commands/build_test.go index 920175fef..e15cebb3e 100644 --- a/pkg/commands/build_test.go +++ b/pkg/commands/build_test.go @@ -104,49 +104,51 @@ func TestBuild(t *testing.T) { } for _, testcaseName := range testcases.List() { - t.Run(testcaseName, func(t *testing.T) { - name := testcaseName - testcase := buildTestCase{} - testcaseDir := filepath.Join("testdata", "testcase-"+name) - testcaseData, err := ioutil.ReadFile(filepath.Join(testcaseDir, "test.yaml")) - if err != nil { - t.Fatalf("%s: %v", name, err) - } - if err := yaml.Unmarshal(testcaseData, &testcase); err != nil { - t.Fatalf("%s: %v", name, err) - } - - ops := &buildOptions{ - kustomizationPath: testcase.Filename, - } - buf := bytes.NewBuffer([]byte{}) - err = ops.RunBuild(buf, os.Stderr, fs) - switch { - case err != nil && len(testcase.ExpectedError) == 0: - t.Errorf("unexpected error: %v", err) - case err != nil && len(testcase.ExpectedError) != 0: - if !strings.Contains(err.Error(), testcase.ExpectedError) { - t.Errorf("expected error to contain %q but got: %v", testcase.ExpectedError, err) - } - return - case err == nil && len(testcase.ExpectedError) != 0: - t.Errorf("unexpected no error") - } - - actualBytes := buf.Bytes() - if !updateKustomizeExpected { - expectedBytes, err := ioutil.ReadFile(testcase.ExpectedStdout) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !reflect.DeepEqual(actualBytes, expectedBytes) { - t.Errorf("%s\ndoesn't equal expected:\n%s\n", actualBytes, expectedBytes) - } - } else { - ioutil.WriteFile(testcase.ExpectedStdout, actualBytes, 0644) - } - - }) + t.Run(testcaseName, func(t *testing.T) { runBuildTestCase(t, testcaseName, updateKustomizeExpected, fs) }) + } + +} + +func runBuildTestCase(t *testing.T, testcaseName string, updateKustomizeExpected bool, fs fs.FileSystem) { + name := testcaseName + testcase := buildTestCase{} + testcaseDir := filepath.Join("testdata", "testcase-"+name) + testcaseData, err := ioutil.ReadFile(filepath.Join(testcaseDir, "test.yaml")) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if err := yaml.Unmarshal(testcaseData, &testcase); err != nil { + t.Fatalf("%s: %v", name, err) + } + + ops := &buildOptions{ + kustomizationPath: testcase.Filename, + } + buf := bytes.NewBuffer([]byte{}) + err = ops.RunBuild(buf, os.Stderr, fs) + switch { + case err != nil && len(testcase.ExpectedError) == 0: + t.Errorf("unexpected error: %v", err) + case err != nil && len(testcase.ExpectedError) != 0: + if !strings.Contains(err.Error(), testcase.ExpectedError) { + t.Errorf("expected error to contain %q but got: %v", testcase.ExpectedError, err) + } + return + case err == nil && len(testcase.ExpectedError) != 0: + t.Errorf("unexpected no error") + } + + actualBytes := buf.Bytes() + if !updateKustomizeExpected { + expectedBytes, err := ioutil.ReadFile(testcase.ExpectedStdout) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if !reflect.DeepEqual(actualBytes, expectedBytes) { + t.Errorf("%s\ndoesn't equal expected:\n%s\n", actualBytes, expectedBytes) + } + } else { + ioutil.WriteFile(testcase.ExpectedStdout, actualBytes, 0644) } } diff --git a/pkg/commands/diff_test.go b/pkg/commands/diff_test.go index ba63f7f33..c5ab00567 100644 --- a/pkg/commands/diff_test.go +++ b/pkg/commands/diff_test.go @@ -74,51 +74,57 @@ func TestDiff(t *testing.T) { for _, testcaseName := range testcases.List() { t.Run(testcaseName, func(t *testing.T) { - name := testcaseName - testcase := DiffTestCase{} - testcaseDir := filepath.Join("testdata", "testcase-"+name) - testcaseData, err := ioutil.ReadFile(filepath.Join(testcaseDir, "test.yaml")) - if err != nil { - t.Fatalf("%s: %v", name, err) - } - if err := yaml.Unmarshal(testcaseData, &testcase); err != nil { - t.Fatalf("%s: %v", name, err) - } - - diffOps := &diffOptions{ - kustomizationPath: testcase.Filename, - } - buf := bytes.NewBuffer([]byte{}) - err = diffOps.RunDiff(buf, os.Stderr, fs) - switch { - case err != nil && len(testcase.ExpectedError) == 0: - t.Errorf("unexpected error: %v", err) - case err != nil && len(testcase.ExpectedError) != 0: - if !strings.Contains(err.Error(), testcase.ExpectedError) { - t.Errorf("expected error to contain %q but got: %v", testcase.ExpectedError, err) - } - return - case err == nil && len(testcase.ExpectedError) != 0: - t.Errorf("unexpected no error") - } - - actualString := string(buf.Bytes()) - actualString = noopDir.ReplaceAllString(actualString, "/tmp/noop/") - actualString = transformedDir.ReplaceAllString(actualString, "/tmp/transformed/") - actualString = timestamp.ReplaceAllString(actualString, "YYYY-MM-DD HH:MM:SS") - actualBytes := []byte(actualString) - if !updateKustomizeExpected { - expectedBytes, err := ioutil.ReadFile(testcase.ExpectedDiff) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if !reflect.DeepEqual(actualBytes, expectedBytes) { - t.Errorf("%s\ndoesn't equal expected:\n%s\n", actualBytes, expectedBytes) - } - } else { - ioutil.WriteFile(testcase.ExpectedDiff, actualBytes, 0644) - } - + runDiffTestCase(t, testcaseName, updateKustomizeExpected, fs, + noopDir, transformedDir, timestamp) }) } } + +func runDiffTestCase(t *testing.T, testcaseName string, updateKustomizeExpected bool, fs fs.FileSystem, + noopDir, transformedDir, timestamp *regexp.Regexp) { + name := testcaseName + testcase := DiffTestCase{} + testcaseDir := filepath.Join("testdata", "testcase-"+name) + testcaseData, err := ioutil.ReadFile(filepath.Join(testcaseDir, "test.yaml")) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if err := yaml.Unmarshal(testcaseData, &testcase); err != nil { + t.Fatalf("%s: %v", name, err) + } + + diffOps := &diffOptions{ + kustomizationPath: testcase.Filename, + } + buf := bytes.NewBuffer([]byte{}) + err = diffOps.RunDiff(buf, os.Stderr, fs) + switch { + case err != nil && len(testcase.ExpectedError) == 0: + t.Errorf("unexpected error: %v", err) + case err != nil && len(testcase.ExpectedError) != 0: + if !strings.Contains(err.Error(), testcase.ExpectedError) { + t.Errorf("expected error to contain %q but got: %v", testcase.ExpectedError, err) + } + return + case err == nil && len(testcase.ExpectedError) != 0: + t.Errorf("unexpected no error") + } + + actualString := string(buf.Bytes()) + actualString = noopDir.ReplaceAllString(actualString, "/tmp/noop/") + actualString = transformedDir.ReplaceAllString(actualString, "/tmp/transformed/") + actualString = timestamp.ReplaceAllString(actualString, "YYYY-MM-DD HH:MM:SS") + actualBytes := []byte(actualString) + if !updateKustomizeExpected { + expectedBytes, err := ioutil.ReadFile(testcase.ExpectedDiff) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if !reflect.DeepEqual(actualBytes, expectedBytes) { + t.Errorf("%s\ndoesn't equal expected:\n%s\n", actualBytes, expectedBytes) + } + } else { + ioutil.WriteFile(testcase.ExpectedDiff, actualBytes, 0644) + } + +}