From 3fcc2edce6f9b32df79f5125184ced17f0936dfd Mon Sep 17 00:00:00 2001 From: Benjamin Elder Date: Thu, 6 Apr 2023 20:55:49 -0700 Subject: [PATCH] cleanup integration test helpers --- hack/tools/require-coverage/main.go | 6 +-- internal/integration/bins.go | 63 ----------------------------- internal/integration/paths.go | 16 +++----- internal/integration/paths_test.go | 27 +++++++++++-- 4 files changed, 31 insertions(+), 81 deletions(-) delete mode 100644 internal/integration/bins.go diff --git a/hack/tools/require-coverage/main.go b/hack/tools/require-coverage/main.go index 5c7aed5..1427534 100644 --- a/hack/tools/require-coverage/main.go +++ b/hack/tools/require-coverage/main.go @@ -48,10 +48,8 @@ var knownFailingFiles = sets.NewString( "k8s.io/registry.k8s.io/cmd/geranos/s3uploader.go", "k8s.io/registry.k8s.io/cmd/geranos/schemav1.go", "k8s.io/registry.k8s.io/cmd/geranos/walkimages.go", - // integration test utilites - "k8s.io/registry.k8s.io/internal/integration/paths.go", - "k8s.io/registry.k8s.io/internal/integration/bins.go", - // TODO: we can cover this + // We cover this with integration tests and including integration coverage + // here would mask a lack of unit test coverage. "k8s.io/registry.k8s.io/cmd/archeio/main.go", ) diff --git a/internal/integration/bins.go b/internal/integration/bins.go deleted file mode 100644 index ecbc53b..0000000 --- a/internal/integration/bins.go +++ /dev/null @@ -1,63 +0,0 @@ -/* -Copyright 2022 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package integration - -import ( - "fmt" - "os" - "os/exec" - "path/filepath" - "strings" -) - -func EnsureBinsInPath(binDir string) error { - path := os.Getenv("PATH") - // if bins are already at front of PATH, do nothing - if strings.HasPrefix(path, binDir+string(os.PathSeparator)) { - return nil - } - // otherwise prepend and set - newPath := binDir + string(os.PathListSeparator) + path - return os.Setenv("PATH", newPath) -} - -// EnsureCrane ensures crane is available in PATH for testing -// under rootPath/bin -// See also: EnsureBinsInPath -func EnsureCrane(rootPath string) error { - // ensure $REPO_ROOT/bin is in the front of $PATH - root, err := ModuleRootDir() - if err != nil { - return fmt.Errorf("failed to detect path to project root: %w", err) - } - binDir := rootToBinDir(root) - if err := EnsureBinsInPath(binDir); err != nil { - return fmt.Errorf("failed to ensure PATH: %w", err) - } - // install crane - // nolint:gosec // we *want* user supplied command arguments ... - cmd := exec.Command( - "go", "build", - "-o", filepath.Join(binDir, "crane"), - "github.com/google/go-containerregistry/cmd/crane", - ) - cmd.Dir = rootToToolsDir(root) - if err := cmd.Run(); err != nil { - return fmt.Errorf("failed to install crane: %w", err) - } - return nil -} diff --git a/internal/integration/paths.go b/internal/integration/paths.go index 4e98e27..e58edcf 100644 --- a/internal/integration/paths.go +++ b/internal/integration/paths.go @@ -23,8 +23,12 @@ import ( ) func ModuleRootDir() (string, error) { + return moduleRootDir(os.Getwd) +} + +func moduleRootDir(getWD func() (string, error)) (string, error) { // in a test, the working directory will be the test package source dir - wd, err := os.Getwd() + wd, err := getWD() if err != nil { return "", err } @@ -35,8 +39,6 @@ func ModuleRootDir() (string, error) { _, err := os.Stat(filepath.Join(currDir, "go.mod")) if err == nil { return currDir, nil - } else if !os.IsNotExist(err) { - return "", err } // if we get back the same path, we've hit the disk / volume root nextDir := filepath.Dir(currDir) @@ -46,11 +48,3 @@ func ModuleRootDir() (string, error) { currDir = nextDir } } - -func rootToBinDir(root string) string { - return filepath.Join(root, "bin") -} - -func rootToToolsDir(root string) string { - return filepath.Join(root, "hack", "tools") -} diff --git a/internal/integration/paths_test.go b/internal/integration/paths_test.go index cbd419e..4831aef 100644 --- a/internal/integration/paths_test.go +++ b/internal/integration/paths_test.go @@ -16,14 +16,35 @@ limitations under the License. package integration -import "testing" +import ( + "errors" + "testing" +) func TestModuleRootDir(t *testing.T) { root, err := ModuleRootDir() if err != nil { t.Fatalf("unexpected error getting root dir: %v", err) + } else if root == "" { + t.Fatal("expected root dir to be non-empty string") } - if root == "" { - t.Fatalf("expected root dir to be non-empty string") + + // we reasonably assume the filesystem root is not a module + wdAlwaysRoot := func() (string, error) { return "/", nil } + root, err = moduleRootDir(wdAlwaysRoot) + if err == nil { + t.Fatal("expected error getting moduleRootDir for /") + } else if root != "" { + t.Fatal("did not expect non-empty string getting moduleRootDir for /") + } + + // test error handling for os.Getwd + expectErr := errors.New("err") + wdAlwaysError := func() (string, error) { return "", expectErr } + root, err = moduleRootDir(wdAlwaysError) + if err == nil { + t.Fatal("expected error getting moduleRootDir with erroring getWD") + } else if root != "" { + t.Fatal("did not expect non-empty string getting moduleRootDir for erroring getWD") } }