From f82ba47e07a00d9b67b037a61a8c03fa81bc37c5 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Tue, 4 Jun 2019 23:47:34 +0200 Subject: [PATCH 01/14] Add util to set up krew integration tests --- integration/krew/krew.go | 102 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 integration/krew/krew.go diff --git a/integration/krew/krew.go b/integration/krew/krew.go new file mode 100644 index 00000000..7278fae8 --- /dev/null +++ b/integration/krew/krew.go @@ -0,0 +1,102 @@ +package krew + +import ( + "context" + "fmt" + "os" + "os/exec" + "testing" + "time" + + "github.com/golang/glog" + "github.com/pkg/errors" + "sigs.k8s.io/krew/pkg/testutil" +) + +// KrewTest is used to set up `krew` integration tests. +type KrewTest struct { + t *testing.T + args []string + env []string + tempDir *testutil.TempDir +} + +// NewKrewTest creates a fluent krew KrewTest +func NewKrewTest(t *testing.T) (*KrewTest, func()) { + tempDir, cleanup := testutil.NewTempDir(t) + return &KrewTest{ + t: t, + env: []string{fmt.Sprintf("KREW_ROOT=%s", tempDir.Root())}, + tempDir: tempDir, + }, cleanup +} + +func (k *KrewTest) Root() string { + return k.tempDir.Root() +} + +// Cmd sets the arguments to krew +func (k *KrewTest) Cmd(args ...string) *KrewTest { + k.args = args + return k +} + +// WithEnv sets an environment variable for the krew run. +func (k *KrewTest) WithEnv(key string, value interface{}) *KrewTest { + if key == "KREW_ROOT" { + glog.V(1).Infoln("Overriding KREW_ROOT in tests is forbidden") + return k + } + k.env = append(k.env, fmt.Sprintf("%s=%v", key, value)) + return k +} + +// RunOrFail runs the krew command and fails the test if the command returns an error. +func (k *KrewTest) RunOrFail() { + k.t.Helper() + if err := k.Run(); err != nil { + k.t.Fatal(err) + } +} + +// Run runs the krew command. +func (k *KrewTest) Run() error { + k.t.Helper() + + cmd := k.cmd(context.Background()) + glog.V(1).Infoln(cmd.Args) + + start := time.Now() + if err := cmd.Run(); err != nil { + return errors.Wrapf(err, "krew %v", k.args) + } + + glog.V(1).Infoln("Ran in", time.Since(start)) + return nil +} + +// RunOrFailOutput runs the krew command and fails the test if the command +// returns an error. It only returns the standard output. +func (k *KrewTest) RunOrFailOutput() []byte { + k.t.Helper() + + cmd := k.cmd(context.Background()) + cmd.Stdout, cmd.Stderr = nil, nil + glog.V(1).Infoln(cmd.Args) + + start := time.Now() + out, err := cmd.Output() + if err != nil { + k.t.Fatalf("krew %v: %v, %s", k.args, err, out) + } + + glog.V(1).Infoln("Ran in", time.Since(start)) + return out +} + +func (k *KrewTest) cmd(ctx context.Context) *exec.Cmd { + cmd := exec.CommandContext(ctx, "krew", k.args...) + cmd.Env = append(os.Environ(), k.env...) + + return cmd +} From 2796058ec4be7c9ca3b11b4a6e25e9c3bf881552 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Wed, 5 Jun 2019 00:41:29 +0200 Subject: [PATCH 02/14] Add utilities to initialize the krew index For faster tests, the index is only cloned once per test run. Additionally, the index is peristently cached between test runs. --- integration/krew/index.go | 81 +++++++++++++++++++++++++++++++++++++++ integration/krew/krew.go | 15 ++++++-- 2 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 integration/krew/index.go diff --git a/integration/krew/index.go b/integration/krew/index.go new file mode 100644 index 00000000..3ac9563a --- /dev/null +++ b/integration/krew/index.go @@ -0,0 +1,81 @@ +package krew + +import ( + "bytes" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "sync" + + "github.com/golang/glog" + "sigs.k8s.io/krew/cmd/krew/cmd" +) + +const ( + persistentIndexCache = "krew-persistent-index-cache" +) + +var ( + once sync.Once + indexTar []byte +) + +// InitializeIndex initializes the krew index in `$root/index` with the actual krew-index. +// It caches the index tree as in-memory tar after the first run. +func (k *KrewTest) initializeIndex() { + once.Do(func() { + persistentCacheFile := filepath.Join(os.TempDir(), persistentIndexCache) + fileInfo, err := os.Stat(persistentCacheFile) + + if err == nil && fileInfo.Mode().IsRegular() { + k.t.Logf("Using persistent index cache from file %q", persistentCacheFile) + if indexTar, err = ioutil.ReadFile(persistentCacheFile); err == nil { + return + } + } + + if indexTar, err = initFromGitClone(); err != nil { + k.t.Fatalf("cannot clone repository: %s", err) + } + + ioutil.WriteFile(persistentCacheFile, indexTar, 0600) + }) + + indexDir := filepath.Join(k.Root(), "index") + if err := os.Mkdir(indexDir, 0777); err != nil { + k.t.Fatal(err) + } + + cmd := exec.Command("tar", "xzf", "-", "-C", indexDir) + cmd.Stdin = bytes.NewReader(indexTar) + if err := cmd.Run(); err != nil { + k.t.Fatalf("cannot restore index from cache: %s", err) + } +} + +func initFromGitClone() ([]byte, error) { + const tarName = "index.tar" + indexRoot, err := ioutil.TempDir("", "krew-index-cache") + if err != nil { + return nil, err + } + defer func() { + err := os.RemoveAll(indexRoot) + glog.V(1).Infoln("cannot remove temporary directory:", err) + }() + + cmd := exec.Command("git", "clone", "--depth=1", "--single-branch", "--no-tags", cmd.IndexURI) + cmd.Dir = indexRoot + if err = cmd.Run(); err != nil { + return nil, err + } + + cmd = exec.Command("tar", "czf", tarName, "-C", "krew-index", ".") + cmd.Dir = indexRoot + if err = cmd.Run(); err != nil { + return nil, err + } + + return ioutil.ReadFile(filepath.Join(indexRoot, tarName)) +} diff --git a/integration/krew/krew.go b/integration/krew/krew.go index 7278fae8..d0e35ce6 100644 --- a/integration/krew/krew.go +++ b/integration/krew/krew.go @@ -21,7 +21,7 @@ type KrewTest struct { tempDir *testutil.TempDir } -// NewKrewTest creates a fluent krew KrewTest +// NewKrewTest creates a fluent krew KrewTest. func NewKrewTest(t *testing.T) (*KrewTest, func()) { tempDir, cleanup := testutil.NewTempDir(t) return &KrewTest{ @@ -31,13 +31,20 @@ func NewKrewTest(t *testing.T) (*KrewTest, func()) { }, cleanup } +// Cmd sets the arguments to krew. +func (k *KrewTest) Cmd(args ...string) *KrewTest { + k.args = args + return k +} + +// Root returns the krew root directory for this test. func (k *KrewTest) Root() string { return k.tempDir.Root() } -// Cmd sets the arguments to krew -func (k *KrewTest) Cmd(args ...string) *KrewTest { - k.args = args +// WithIndex initializes the index with the actual krew-index from github/kubernetes-sigs/krew-index. +func (k *KrewTest) WithIndex() *KrewTest { + k.initializeIndex() return k } From d6aff99355073537c789ab58581c16d2526403a5 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Wed, 5 Jun 2019 02:19:31 +0200 Subject: [PATCH 03/14] Add simple integration tests to cover basic krew commands --- .travis.yml | 2 +- integration/integration_test.go | 64 +++++++++++++++++++++++++++++++++ integration/krew/index.go | 4 +++ integration/krew/krew.go | 11 +++++- pkg/testutil/tempdir.go | 15 ++++++++ 5 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 integration/integration_test.go diff --git a/.travis.yml b/.travis.yml index 121f0956..7189b9ab 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,7 @@ script: - hack/verify-boilerplate.sh - hack/verify-gofmt.sh - hack/run-lint.sh -- go test -v -coverprofile=coverage.txt -covermode=atomic ./... +- go test -v -short -coverprofile=coverage.txt -covermode=atomic ./... - hack/make-all.sh after_success: - bash <(curl -s https://codecov.io/bash) diff --git a/integration/integration_test.go b/integration/integration_test.go new file mode 100644 index 00000000..ddc74c43 --- /dev/null +++ b/integration/integration_test.go @@ -0,0 +1,64 @@ +package integration + +import ( + "testing" + + "sigs.k8s.io/krew/integration/krew" +) + +const ( + // validPlugin is a valid plugin with a small download size + validPlugin = "konfig" +) + +func TestInstall(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + krewTest, cleanup := krew.NewKrewTest(t) + defer cleanup() + + krewTest.WithIndex().Cmd("install", validPlugin).RunOrFail() +} + +func TestUpdate(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + krewTest, cleanup := krew.NewKrewTest(t) + defer cleanup() + + krewTest.WithIndex().Cmd("update").RunOrFail() + + indexFiles, err := krewTest.TempDir().List("index") + if err != nil { + t.Error(err) + } + + if len(indexFiles) == 0 { + t.Error("expected some index files but found none") + } +} + +func TestUninstall(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + krewTest, cleanup := krew.NewKrewTest(t) + defer cleanup() + + krewTest.WithIndex().Cmd("install", validPlugin).RunOrFailOutput() + krewTest.Cmd("remove", validPlugin).RunOrFailOutput() + + indexFiles, err := krewTest.TempDir().List("store") + if err != nil { + t.Error(err) + } + + if len(indexFiles) != 0 { + t.Error("expected the store to be empty") + } +} diff --git a/integration/krew/index.go b/integration/krew/index.go index 3ac9563a..e528cdb1 100644 --- a/integration/krew/index.go +++ b/integration/krew/index.go @@ -44,6 +44,10 @@ func (k *KrewTest) initializeIndex() { indexDir := filepath.Join(k.Root(), "index") if err := os.Mkdir(indexDir, 0777); err != nil { + if os.IsExist(err) { + k.t.Log("initializeIndex should only be called once") + return + } k.t.Fatal(err) } diff --git a/integration/krew/krew.go b/integration/krew/krew.go index d0e35ce6..78235008 100644 --- a/integration/krew/krew.go +++ b/integration/krew/krew.go @@ -93,6 +93,7 @@ func (k *KrewTest) RunOrFailOutput() []byte { start := time.Now() out, err := cmd.Output() + k.t.Logf("krew %v: %v, %s", k.args, err, out) if err != nil { k.t.Fatalf("krew %v: %v, %s", k.args, err, out) } @@ -102,8 +103,16 @@ func (k *KrewTest) RunOrFailOutput() []byte { } func (k *KrewTest) cmd(ctx context.Context) *exec.Cmd { - cmd := exec.CommandContext(ctx, "krew", k.args...) + args := make([]string, 0, len(k.args)+1) + args = append(args, "krew") + args = append(args, k.args...) + + cmd := exec.CommandContext(ctx, "kubectl", args...) cmd.Env = append(os.Environ(), k.env...) return cmd } + +func (k *KrewTest) TempDir() *testutil.TempDir { + return k.tempDir +} diff --git a/pkg/testutil/tempdir.go b/pkg/testutil/tempdir.go index f0b226ec..e6b8e855 100644 --- a/pkg/testutil/tempdir.go +++ b/pkg/testutil/tempdir.go @@ -70,3 +70,18 @@ func (td *TempDir) Write(file string, content []byte) *TempDir { } return td } + +// List lists all the files in the subpath of the temp directory. +// The input path is expected to use '/' as directory separator regardless of the host OS. +func (td *TempDir) List(path string) ([]string, error) { + var files []string + + err := filepath.Walk(td.Path(path), func(path string, info os.FileInfo, err error) error { + if err == nil && info.Mode().IsRegular() { + files = append(files, filepath.ToSlash(path)) + } + return err + }) + + return files, err +} From 1c3b1363c6620023f351fcc678ca3fd1148eff72 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 12:52:35 +0200 Subject: [PATCH 04/14] Review comments --- integration/integration_test.go | 19 ++++++++++--------- integration/krew/krew.go | 4 +--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index ddc74c43..c609756d 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -12,9 +12,7 @@ const ( ) func TestInstall(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } + skipShort(t) krewTest, cleanup := krew.NewKrewTest(t) defer cleanup() @@ -23,9 +21,7 @@ func TestInstall(t *testing.T) { } func TestUpdate(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } + skipShort(t) krewTest, cleanup := krew.NewKrewTest(t) defer cleanup() @@ -43,9 +39,7 @@ func TestUpdate(t *testing.T) { } func TestUninstall(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } + skipShort(t) krewTest, cleanup := krew.NewKrewTest(t) defer cleanup() @@ -62,3 +56,10 @@ func TestUninstall(t *testing.T) { t.Error("expected the store to be empty") } } + +func skipShort(t *testing.T) { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test") + } +} diff --git a/integration/krew/krew.go b/integration/krew/krew.go index 78235008..1fc0cbc0 100644 --- a/integration/krew/krew.go +++ b/integration/krew/krew.go @@ -88,12 +88,10 @@ func (k *KrewTest) RunOrFailOutput() []byte { k.t.Helper() cmd := k.cmd(context.Background()) - cmd.Stdout, cmd.Stderr = nil, nil glog.V(1).Infoln(cmd.Args) start := time.Now() - out, err := cmd.Output() - k.t.Logf("krew %v: %v, %s", k.args, err, out) + out, err := cmd.CombinedOutput() if err != nil { k.t.Fatalf("krew %v: %v, %s", k.args, err, out) } From 8e46e9acfac72af947f14a7a0afa102bee993fc4 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 12:54:17 +0200 Subject: [PATCH 05/14] Remove flawed test cases --- integration/integration_test.go | 38 +-------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index c609756d..fd5517f5 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -18,43 +18,7 @@ func TestInstall(t *testing.T) { defer cleanup() krewTest.WithIndex().Cmd("install", validPlugin).RunOrFail() -} - -func TestUpdate(t *testing.T) { - skipShort(t) - - krewTest, cleanup := krew.NewKrewTest(t) - defer cleanup() - - krewTest.WithIndex().Cmd("update").RunOrFail() - - indexFiles, err := krewTest.TempDir().List("index") - if err != nil { - t.Error(err) - } - - if len(indexFiles) == 0 { - t.Error("expected some index files but found none") - } -} - -func TestUninstall(t *testing.T) { - skipShort(t) - - krewTest, cleanup := krew.NewKrewTest(t) - defer cleanup() - - krewTest.WithIndex().Cmd("install", validPlugin).RunOrFailOutput() - krewTest.Cmd("remove", validPlugin).RunOrFailOutput() - - indexFiles, err := krewTest.TempDir().List("store") - if err != nil { - t.Error(err) - } - - if len(indexFiles) != 0 { - t.Error("expected the store to be empty") - } + // todo(corneliusweig): make sure that the plugin can be executed as `kubectl konfig --help` } func skipShort(t *testing.T) { From 435580a22a1b441e22a4047f34fd23cdb616ecf0 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 13:24:20 +0200 Subject: [PATCH 06/14] Add basic integration test for `krew help` --- integration/integration_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index fd5517f5..74453498 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -11,16 +11,25 @@ const ( validPlugin = "konfig" ) -func TestInstall(t *testing.T) { +func TestKrewInstall(t *testing.T) { skipShort(t) krewTest, cleanup := krew.NewKrewTest(t) defer cleanup() - krewTest.WithIndex().Cmd("install", validPlugin).RunOrFail() + krewTest.WithIndex().Cmd("install", validPlugin).RunOrFailOutput() // todo(corneliusweig): make sure that the plugin can be executed as `kubectl konfig --help` } +func TestKrewHelp(t *testing.T) { + skipShort(t) + + krewTest, cleanup := krew.NewKrewTest(t) + defer cleanup() + + krewTest.Cmd("help").RunOrFail() +} + func skipShort(t *testing.T) { t.Helper() if testing.Short() { From d109a461561683074f30a2b1a8058fa124e34f85 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 13:43:59 +0200 Subject: [PATCH 07/14] Setup PATH variable in tests so that installed plugins can be executed Also, optionally link to a krew binary as specified via the KREW_BINARY env varible. --- integration/krew/krew.go | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/integration/krew/krew.go b/integration/krew/krew.go index 1fc0cbc0..fabcd29b 100644 --- a/integration/krew/krew.go +++ b/integration/krew/krew.go @@ -13,6 +13,8 @@ import ( "sigs.k8s.io/krew/pkg/testutil" ) +const krewBinaryEnv = "KREW_BINARY" + // KrewTest is used to set up `krew` integration tests. type KrewTest struct { t *testing.T @@ -24,13 +26,36 @@ type KrewTest struct { // NewKrewTest creates a fluent krew KrewTest. func NewKrewTest(t *testing.T) (*KrewTest, func()) { tempDir, cleanup := testutil.NewTempDir(t) + pathEnv := setupPathEnv(t, tempDir) return &KrewTest{ t: t, - env: []string{fmt.Sprintf("KREW_ROOT=%s", tempDir.Root())}, + env: []string{pathEnv, fmt.Sprintf("KREW_ROOT=%s", tempDir.Root())}, tempDir: tempDir, }, cleanup } +func setupPathEnv(t *testing.T, tempDir *testutil.TempDir) string { + krewBinPath := tempDir.Path("bin") + if err := os.MkdirAll(krewBinPath, os.ModePerm); err != nil { + t.Fatal(err) + } + + if krewBinary, found := os.LookupEnv(krewBinaryEnv); found { + if err := os.Symlink(krewBinary, tempDir.Path("bin/kubectl-krew")); err != nil { + t.Fatalf("Cannot link to krew: %s", err) + } + } else { + t.Logf("Environment variable %q was not found, using krew installation from host", krewBinaryEnv) + } + + path, found := os.LookupEnv("PATH") + if !found { + t.Fatalf("PATH variable is not set up") + } + + return fmt.Sprintf("PATH=%s:%s", krewBinPath, path) +} + // Cmd sets the arguments to krew. func (k *KrewTest) Cmd(args ...string) *KrewTest { k.args = args From e901b7d1d35d0fd4ea81a218cc7b6d3a8879ca57 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 14:13:02 +0200 Subject: [PATCH 08/14] Make it possible to call plugins other than `krew` --- integration/integration_test.go | 10 +++++----- integration/krew/krew.go | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index 74453498..46e7f5a2 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -14,20 +14,20 @@ const ( func TestKrewInstall(t *testing.T) { skipShort(t) - krewTest, cleanup := krew.NewKrewTest(t) + test, cleanup := krew.NewKrewTest(t) defer cleanup() - krewTest.WithIndex().Cmd("install", validPlugin).RunOrFailOutput() - // todo(corneliusweig): make sure that the plugin can be executed as `kubectl konfig --help` + test.WithIndex().Krew("install", validPlugin).RunOrFailOutput() + test.Call(validPlugin, "--help").RunOrFail() } func TestKrewHelp(t *testing.T) { skipShort(t) - krewTest, cleanup := krew.NewKrewTest(t) + test, cleanup := krew.NewKrewTest(t) defer cleanup() - krewTest.Cmd("help").RunOrFail() + test.Krew("help").RunOrFail() } func skipShort(t *testing.T) { diff --git a/integration/krew/krew.go b/integration/krew/krew.go index fabcd29b..8085fb6d 100644 --- a/integration/krew/krew.go +++ b/integration/krew/krew.go @@ -18,6 +18,7 @@ const krewBinaryEnv = "KREW_BINARY" // KrewTest is used to set up `krew` integration tests. type KrewTest struct { t *testing.T + plugin string args []string env []string tempDir *testutil.TempDir @@ -56,8 +57,16 @@ func setupPathEnv(t *testing.T, tempDir *testutil.TempDir) string { return fmt.Sprintf("PATH=%s:%s", krewBinPath, path) } -// Cmd sets the arguments to krew. -func (k *KrewTest) Cmd(args ...string) *KrewTest { +// Call configures the runner to call plugin with arguments args. +func (k *KrewTest) Call(plugin string, args ...string) *KrewTest { + k.plugin = plugin + k.args = args + return k +} + +// Krew configures the runner to call krew with arguments args. +func (k *KrewTest) Krew(args ...string) *KrewTest { + k.plugin = "krew" k.args = args return k } @@ -127,7 +136,7 @@ func (k *KrewTest) RunOrFailOutput() []byte { func (k *KrewTest) cmd(ctx context.Context) *exec.Cmd { args := make([]string, 0, len(k.args)+1) - args = append(args, "krew") + args = append(args, k.plugin) args = append(args, k.args...) cmd := exec.CommandContext(ctx, "kubectl", args...) From 14c10ad382a317b716128e8fc9d578f29dda8227 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 14:15:45 +0200 Subject: [PATCH 09/14] Enable integration tests on travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 7189b9ab..3455dd1a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,6 +15,7 @@ script: - hack/run-lint.sh - go test -v -short -coverprofile=coverage.txt -covermode=atomic ./... - hack/make-all.sh +- KREW_BINARY=$PWD/../out/bin/krew-linux_amd64 go test -v ./... after_success: - bash <(curl -s https://codecov.io/bash) env: From 46fba23cd6548d80f4b37e72f07ee608efa9ef68 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 14:26:43 +0200 Subject: [PATCH 10/14] Fix boilerplate header --- integration/integration_test.go | 14 ++++++++++++++ integration/krew/index.go | 14 ++++++++++++++ integration/krew/krew.go | 14 ++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/integration/integration_test.go b/integration/integration_test.go index 46e7f5a2..91d18447 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -1,3 +1,17 @@ +// Copyright 2019 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 ( diff --git a/integration/krew/index.go b/integration/krew/index.go index e528cdb1..b5430dc6 100644 --- a/integration/krew/index.go +++ b/integration/krew/index.go @@ -1,3 +1,17 @@ +// Copyright 2019 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 krew import ( diff --git a/integration/krew/krew.go b/integration/krew/krew.go index 8085fb6d..67fbdfc0 100644 --- a/integration/krew/krew.go +++ b/integration/krew/krew.go @@ -1,3 +1,17 @@ +// Copyright 2019 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 krew import ( From 2be4b0744cc80de5411ac09decafafe3be4ed6f8 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 21:11:26 +0200 Subject: [PATCH 11/14] Move IndexURI to constants This is important, because importing the URI in integration tests triggers the init functions in the cmd package. This has very nasty and hard to find side-effects. --- cmd/krew/cmd/update.go | 9 +++------ cmd/krew/cmd/version.go | 3 ++- integration/krew/index.go | 4 ++-- pkg/constants/constants.go | 3 +++ 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cmd/krew/cmd/update.go b/cmd/krew/cmd/update.go index 995d05a3..02622969 100644 --- a/cmd/krew/cmd/update.go +++ b/cmd/krew/cmd/update.go @@ -19,15 +19,12 @@ import ( "os" "github.com/golang/glog" - "sigs.k8s.io/krew/pkg/gitutil" - "github.com/pkg/errors" "github.com/spf13/cobra" + "sigs.k8s.io/krew/pkg/constants" + "sigs.k8s.io/krew/pkg/gitutil" ) -// IndexURI points to the upstream index. -const IndexURI = "https://github.com/kubernetes-sigs/krew-index.git" - // updateCmd represents the update command var updateCmd = &cobra.Command{ Use: "update", @@ -45,7 +42,7 @@ Remarks: func ensureIndexUpdated(_ *cobra.Command, _ []string) error { glog.V(1).Infof("Updating the local copy of plugin index (%s)", paths.IndexPath()) - if err := gitutil.EnsureUpdated(IndexURI, paths.IndexPath()); err != nil { + if err := gitutil.EnsureUpdated(constants.IndexURI, paths.IndexPath()); err != nil { return errors.Wrap(err, "failed to update the local index") } fmt.Fprintln(os.Stderr, "Updated the local copy of plugin index.") diff --git a/cmd/krew/cmd/version.go b/cmd/krew/cmd/version.go index 9e66591d..3aa25cd9 100644 --- a/cmd/krew/cmd/version.go +++ b/cmd/krew/cmd/version.go @@ -21,6 +21,7 @@ import ( "github.com/golang/glog" "github.com/pkg/errors" "github.com/spf13/cobra" + "sigs.k8s.io/krew/pkg/constants" "sigs.k8s.io/krew/pkg/environment" "sigs.k8s.io/krew/pkg/version" ) @@ -58,7 +59,7 @@ Remarks: {"ExecutedVersion", executedVersion}, {"GitTag", version.GitTag()}, {"GitCommit", version.GitCommit()}, - {"IndexURI", IndexURI}, + {"IndexURI", constants.IndexURI}, {"BasePath", paths.BasePath()}, {"IndexPath", paths.IndexPath()}, {"InstallPath", paths.InstallPath()}, diff --git a/integration/krew/index.go b/integration/krew/index.go index b5430dc6..331a69bd 100644 --- a/integration/krew/index.go +++ b/integration/krew/index.go @@ -23,7 +23,7 @@ import ( "sync" "github.com/golang/glog" - "sigs.k8s.io/krew/cmd/krew/cmd" + "sigs.k8s.io/krew/pkg/constants" ) const ( @@ -83,7 +83,7 @@ func initFromGitClone() ([]byte, error) { glog.V(1).Infoln("cannot remove temporary directory:", err) }() - cmd := exec.Command("git", "clone", "--depth=1", "--single-branch", "--no-tags", cmd.IndexURI) + cmd := exec.Command("git", "clone", "--depth=1", "--single-branch", "--no-tags", constants.IndexURI) cmd.Dir = indexRoot if err = cmd.Run(); err != nil { return nil, err diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index a1e9c0cd..66f93b64 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -17,4 +17,7 @@ package constants const ( CurrentAPIVersion = "krew.googlecontainertools.github.com/v1alpha2" PluginKind = "Plugin" + + // IndexURI points to the upstream index. + IndexURI = "https://github.com/kubernetes-sigs/krew-index.git" ) From 76cb3551cd5e1c330ddda0dad72f76495e16a5dd Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 21:23:10 +0200 Subject: [PATCH 12/14] Review comments - rename package integration -> test - disable integration tests in run-tests.sh - remove obsolete tempdir helper --- hack/run-tests.sh | 2 +- pkg/testutil/tempdir.go | 15 --------------- {integration => test}/krew/index.go | 0 {integration => test}/krew/krew.go | 0 .../integration_test.go => test/krew_test.go | 5 +++-- 5 files changed, 4 insertions(+), 18 deletions(-) rename {integration => test}/krew/index.go (100%) rename {integration => test}/krew/krew.go (100%) rename integration/integration_test.go => test/krew_test.go (92%) diff --git a/hack/run-tests.sh b/hack/run-tests.sh index 6bc1c339..3c06f53a 100755 --- a/hack/run-tests.sh +++ b/hack/run-tests.sh @@ -44,7 +44,7 @@ print_with_color "$color_blue" 'Running gofmt' "$SCRIPTDIR"/verify-gofmt.sh print_with_color "$color_blue" 'Running tests' -go test -v -race sigs.k8s.io/krew/... +go test -v -short -race sigs.k8s.io/krew/... print_with_color "$color_blue" 'Running linter' "$SCRIPTDIR"/run-lint.sh diff --git a/pkg/testutil/tempdir.go b/pkg/testutil/tempdir.go index e6b8e855..f0b226ec 100644 --- a/pkg/testutil/tempdir.go +++ b/pkg/testutil/tempdir.go @@ -70,18 +70,3 @@ func (td *TempDir) Write(file string, content []byte) *TempDir { } return td } - -// List lists all the files in the subpath of the temp directory. -// The input path is expected to use '/' as directory separator regardless of the host OS. -func (td *TempDir) List(path string) ([]string, error) { - var files []string - - err := filepath.Walk(td.Path(path), func(path string, info os.FileInfo, err error) error { - if err == nil && info.Mode().IsRegular() { - files = append(files, filepath.ToSlash(path)) - } - return err - }) - - return files, err -} diff --git a/integration/krew/index.go b/test/krew/index.go similarity index 100% rename from integration/krew/index.go rename to test/krew/index.go diff --git a/integration/krew/krew.go b/test/krew/krew.go similarity index 100% rename from integration/krew/krew.go rename to test/krew/krew.go diff --git a/integration/integration_test.go b/test/krew_test.go similarity index 92% rename from integration/integration_test.go rename to test/krew_test.go index 91d18447..410c64ac 100644 --- a/integration/integration_test.go +++ b/test/krew_test.go @@ -12,12 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -package integration +// Package test contains integration tests for krew. +package test import ( "testing" - "sigs.k8s.io/krew/integration/krew" + "sigs.k8s.io/krew/test/krew" ) const ( From 1ddfaa823df01e42155e48f1f2c7722f0b8c2da5 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 22:19:21 +0200 Subject: [PATCH 13/14] Enable integration tests on travis - Use a script run-integration-tests.sh for that - Add instructions how to run this script locally --- .travis.yml | 2 +- docs/CONTRIBUTOR_GUIDE.md | 19 ++++++++++++++ hack/run-integration-tests.sh | 49 +++++++++++++++++++++++++++++++++++ hack/run-lint.sh | 2 +- 4 files changed, 70 insertions(+), 2 deletions(-) create mode 100755 hack/run-integration-tests.sh diff --git a/.travis.yml b/.travis.yml index 3455dd1a..0d3f871e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ script: - hack/run-lint.sh - go test -v -short -coverprofile=coverage.txt -covermode=atomic ./... - hack/make-all.sh -- KREW_BINARY=$PWD/../out/bin/krew-linux_amd64 go test -v ./... +- hack/run-integration-tests.sh after_success: - bash <(curl -s https://codecov.io/bash) env: diff --git a/docs/CONTRIBUTOR_GUIDE.md b/docs/CONTRIBUTOR_GUIDE.md index c0aff06f..dfa910a1 100644 --- a/docs/CONTRIBUTOR_GUIDE.md +++ b/docs/CONTRIBUTOR_GUIDE.md @@ -47,9 +47,28 @@ To run tests locally, the easiest way to get started is with hack/run-tests.sh ``` +This will run all unit tests and code quality tools. To run a single tool independently of the other code checks, have a look at the other scripts in [`hack/`](../hack). +In addition, there are integration tests to cover high-level krew functionality. +To run integration tests, you will need to build the `krew` binary beforehand: + +```bash +hack/make-binaries.sh +hack/run-integration-tests.sh +``` + +The above builds binaries for all supported platforms. +Building only for your platform produces a slightly different image but will +work in most circumstances: + +```bash +go build ./cmd/krew +# you need to specify your local krew binary when using this method: +hack/run-integration-tests.sh ./krew +``` + ## Testing `krew` in a sandbox After making changes to krew, you should also check that it behaves as expected. diff --git a/hack/run-integration-tests.sh b/hack/run-integration-tests.sh new file mode 100755 index 00000000..b6aa6648 --- /dev/null +++ b/hack/run-integration-tests.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash + +# Copyright 2019 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. + +set -euo pipefail + +SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +BINDIR="${SCRIPTDIR}/../out/bin" +KREW_BINARY_DEFAULT="$BINDIR/krew-linux_amd64" + +if [[ "$#" -gt 0 && ( "$1" = '-h' || "$1" = '--help' ) ]]; then + cat </dev/null; then + echo 'using kubectl from the host system' + else + # install kubectl + local -r KUBECTL_VERSION='v1.14.2' + local -r KUBECTL_BINARY="$BINDIR/kubectl" + curl -fSsLo "$KUBECTL_BINARY" https://storage.googleapis.com/kubernetes-release/release/${KUBECTL_VERSION}/bin/linux/amd64/kubectl + chmod +x "$KUBECTL_BINARY" + export PATH="$BINDIR:$PATH" + fi +} + +install_kubectl_if_needed +KREW_BINARY=$(readlink -f "${1:-$KREW_BINARY_DEFAULT}") # needed for `kubectl krew` in tests +export KREW_BINARY + +go test -v ./... diff --git a/hack/run-lint.sh b/hack/run-lint.sh index ff61abf3..95710c61 100755 --- a/hack/run-lint.sh +++ b/hack/run-lint.sh @@ -16,7 +16,7 @@ set -euo pipefail -HACK=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +SCRIPTDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) if ! [[ -x "$GOPATH/bin/golangci-lint" ]] then From bb482248ac49470c13ec6bfd422ad4140ad004f9 Mon Sep 17 00:00:00 2001 From: Cornelius Weig Date: Thu, 6 Jun 2019 22:30:12 +0200 Subject: [PATCH 14/14] Rename test types to avoid stuttering --- test/krew/index.go | 14 ++++---- test/krew/krew.go | 82 +++++++++++++++++++++++----------------------- test/krew_test.go | 4 +-- 3 files changed, 50 insertions(+), 50 deletions(-) diff --git a/test/krew/index.go b/test/krew/index.go index 331a69bd..b727c18e 100644 --- a/test/krew/index.go +++ b/test/krew/index.go @@ -37,38 +37,38 @@ var ( // InitializeIndex initializes the krew index in `$root/index` with the actual krew-index. // It caches the index tree as in-memory tar after the first run. -func (k *KrewTest) initializeIndex() { +func (it *ITest) initializeIndex() { once.Do(func() { persistentCacheFile := filepath.Join(os.TempDir(), persistentIndexCache) fileInfo, err := os.Stat(persistentCacheFile) if err == nil && fileInfo.Mode().IsRegular() { - k.t.Logf("Using persistent index cache from file %q", persistentCacheFile) + it.t.Logf("Using persistent index cache from file %q", persistentCacheFile) if indexTar, err = ioutil.ReadFile(persistentCacheFile); err == nil { return } } if indexTar, err = initFromGitClone(); err != nil { - k.t.Fatalf("cannot clone repository: %s", err) + it.t.Fatalf("cannot clone repository: %s", err) } ioutil.WriteFile(persistentCacheFile, indexTar, 0600) }) - indexDir := filepath.Join(k.Root(), "index") + indexDir := filepath.Join(it.Root(), "index") if err := os.Mkdir(indexDir, 0777); err != nil { if os.IsExist(err) { - k.t.Log("initializeIndex should only be called once") + it.t.Log("initializeIndex should only be called once") return } - k.t.Fatal(err) + it.t.Fatal(err) } cmd := exec.Command("tar", "xzf", "-", "-C", indexDir) cmd.Stdin = bytes.NewReader(indexTar) if err := cmd.Run(); err != nil { - k.t.Fatalf("cannot restore index from cache: %s", err) + it.t.Fatalf("cannot restore index from cache: %s", err) } } diff --git a/test/krew/krew.go b/test/krew/krew.go index 67fbdfc0..afe55241 100644 --- a/test/krew/krew.go +++ b/test/krew/krew.go @@ -29,8 +29,8 @@ import ( const krewBinaryEnv = "KREW_BINARY" -// KrewTest is used to set up `krew` integration tests. -type KrewTest struct { +// ITest is used to set up `krew` integration tests. +type ITest struct { t *testing.T plugin string args []string @@ -38,11 +38,11 @@ type KrewTest struct { tempDir *testutil.TempDir } -// NewKrewTest creates a fluent krew KrewTest. -func NewKrewTest(t *testing.T) (*KrewTest, func()) { +// NewTest creates a fluent krew ITest. +func NewTest(t *testing.T) (*ITest, func()) { tempDir, cleanup := testutil.NewTempDir(t) pathEnv := setupPathEnv(t, tempDir) - return &KrewTest{ + return &ITest{ t: t, env: []string{pathEnv, fmt.Sprintf("KREW_ROOT=%s", tempDir.Root())}, tempDir: tempDir, @@ -72,58 +72,58 @@ func setupPathEnv(t *testing.T, tempDir *testutil.TempDir) string { } // Call configures the runner to call plugin with arguments args. -func (k *KrewTest) Call(plugin string, args ...string) *KrewTest { - k.plugin = plugin - k.args = args - return k +func (it *ITest) Call(plugin string, args ...string) *ITest { + it.plugin = plugin + it.args = args + return it } // Krew configures the runner to call krew with arguments args. -func (k *KrewTest) Krew(args ...string) *KrewTest { - k.plugin = "krew" - k.args = args - return k +func (it *ITest) Krew(args ...string) *ITest { + it.plugin = "krew" + it.args = args + return it } // Root returns the krew root directory for this test. -func (k *KrewTest) Root() string { - return k.tempDir.Root() +func (it *ITest) Root() string { + return it.tempDir.Root() } // WithIndex initializes the index with the actual krew-index from github/kubernetes-sigs/krew-index. -func (k *KrewTest) WithIndex() *KrewTest { - k.initializeIndex() - return k +func (it *ITest) WithIndex() *ITest { + it.initializeIndex() + return it } // WithEnv sets an environment variable for the krew run. -func (k *KrewTest) WithEnv(key string, value interface{}) *KrewTest { +func (it *ITest) WithEnv(key string, value interface{}) *ITest { if key == "KREW_ROOT" { glog.V(1).Infoln("Overriding KREW_ROOT in tests is forbidden") - return k + return it } - k.env = append(k.env, fmt.Sprintf("%s=%v", key, value)) - return k + it.env = append(it.env, fmt.Sprintf("%s=%v", key, value)) + return it } // RunOrFail runs the krew command and fails the test if the command returns an error. -func (k *KrewTest) RunOrFail() { - k.t.Helper() - if err := k.Run(); err != nil { - k.t.Fatal(err) +func (it *ITest) RunOrFail() { + it.t.Helper() + if err := it.Run(); err != nil { + it.t.Fatal(err) } } // Run runs the krew command. -func (k *KrewTest) Run() error { - k.t.Helper() +func (it *ITest) Run() error { + it.t.Helper() - cmd := k.cmd(context.Background()) + cmd := it.cmd(context.Background()) glog.V(1).Infoln(cmd.Args) start := time.Now() if err := cmd.Run(); err != nil { - return errors.Wrapf(err, "krew %v", k.args) + return errors.Wrapf(err, "krew %v", it.args) } glog.V(1).Infoln("Ran in", time.Since(start)) @@ -132,33 +132,33 @@ func (k *KrewTest) Run() error { // RunOrFailOutput runs the krew command and fails the test if the command // returns an error. It only returns the standard output. -func (k *KrewTest) RunOrFailOutput() []byte { - k.t.Helper() +func (it *ITest) RunOrFailOutput() []byte { + it.t.Helper() - cmd := k.cmd(context.Background()) + cmd := it.cmd(context.Background()) glog.V(1).Infoln(cmd.Args) start := time.Now() out, err := cmd.CombinedOutput() if err != nil { - k.t.Fatalf("krew %v: %v, %s", k.args, err, out) + it.t.Fatalf("krew %v: %v, %s", it.args, err, out) } glog.V(1).Infoln("Ran in", time.Since(start)) return out } -func (k *KrewTest) cmd(ctx context.Context) *exec.Cmd { - args := make([]string, 0, len(k.args)+1) - args = append(args, k.plugin) - args = append(args, k.args...) +func (it *ITest) cmd(ctx context.Context) *exec.Cmd { + args := make([]string, 0, len(it.args)+1) + args = append(args, it.plugin) + args = append(args, it.args...) cmd := exec.CommandContext(ctx, "kubectl", args...) - cmd.Env = append(os.Environ(), k.env...) + cmd.Env = append(os.Environ(), it.env...) return cmd } -func (k *KrewTest) TempDir() *testutil.TempDir { - return k.tempDir +func (it *ITest) TempDir() *testutil.TempDir { + return it.tempDir } diff --git a/test/krew_test.go b/test/krew_test.go index 410c64ac..8f95ba93 100644 --- a/test/krew_test.go +++ b/test/krew_test.go @@ -29,7 +29,7 @@ const ( func TestKrewInstall(t *testing.T) { skipShort(t) - test, cleanup := krew.NewKrewTest(t) + test, cleanup := krew.NewTest(t) defer cleanup() test.WithIndex().Krew("install", validPlugin).RunOrFailOutput() @@ -39,7 +39,7 @@ func TestKrewInstall(t *testing.T) { func TestKrewHelp(t *testing.T) { skipShort(t) - test, cleanup := krew.NewKrewTest(t) + test, cleanup := krew.NewTest(t) defer cleanup() test.Krew("help").RunOrFail()