From c095660c1cb040a1eb310c00fcc9f28a9e1d2e44 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 28 Sep 2020 12:59:19 +0200 Subject: [PATCH 1/7] pkg/terraform: handle error from os.RemoveAll() in tests Also use t.Cleanup() instead of defer to cleanup after tests. Signed-off-by: Mateusz Gozdek --- pkg/terraform/executor_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/terraform/executor_test.go b/pkg/terraform/executor_test.go index 1baad2c66..1dc6122c7 100644 --- a/pkg/terraform/executor_test.go +++ b/pkg/terraform/executor_test.go @@ -15,7 +15,11 @@ func executor(t *testing.T) *Executor { t.Fatalf("Creating tmp dir should succeed, got: %v", err) } - defer os.RemoveAll(tmpDir) + t.Cleanup(func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Logf("Removing directory %q: %v", tmpDir, err) + } + }) conf := Config{ Verbose: false, From 42042e1a297ed48378bbf345c33353c3aa1bf980 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 28 Sep 2020 13:02:47 +0200 Subject: [PATCH 2/7] pkg/terraform: convert e2e test to use exported API Signed-off-by: Mateusz Gozdek --- pkg/terraform/executor_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/terraform/executor_test.go b/pkg/terraform/executor_test.go index 1dc6122c7..91312d0c5 100644 --- a/pkg/terraform/executor_test.go +++ b/pkg/terraform/executor_test.go @@ -1,15 +1,17 @@ //+build e2e -package terraform +package terraform_test import ( "io/ioutil" "os" "strings" "testing" + + "github.com/kinvolk/lokomotive/pkg/terraform" ) -func executor(t *testing.T) *Executor { +func executor(t *testing.T) *terraform.Executor { tmpDir, err := ioutil.TempDir("", "lokoctl-tests-") if err != nil { t.Fatalf("Creating tmp dir should succeed, got: %v", err) @@ -21,12 +23,12 @@ func executor(t *testing.T) *Executor { } }) - conf := Config{ + conf := terraform.Config{ Verbose: false, WorkingDir: tmpDir, } - ex, err := NewExecutor(conf) + ex, err := terraform.NewExecutor(conf) if err != nil { t.Fatalf("Creating new executor should succeed, got: %v", err) } From bc749c51cd29a456d254353b8ebf15e2f0bcccce Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 28 Sep 2020 13:03:32 +0200 Subject: [PATCH 3/7] pkg/terraform: move e2e tests to separate file So regular unit tests for executor can be run. Signed-off-by: Mateusz Gozdek --- pkg/terraform/{executor_test.go => executor_e2e_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename pkg/terraform/{executor_test.go => executor_e2e_test.go} (100%) diff --git a/pkg/terraform/executor_test.go b/pkg/terraform/executor_e2e_test.go similarity index 100% rename from pkg/terraform/executor_test.go rename to pkg/terraform/executor_e2e_test.go From 5c31626ae98caf9ea09e3a44af0c62d38dba08a6 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 28 Sep 2020 15:09:09 +0200 Subject: [PATCH 4/7] pkg/terraform: test requiredVersion const in unit tests Rather than in runtime. Signed-off-by: Mateusz Gozdek --- pkg/terraform/executor.go | 6 ++---- pkg/terraform/executor_internal_test.go | 27 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 pkg/terraform/executor_internal_test.go diff --git a/pkg/terraform/executor.go b/pkg/terraform/executor.go index 26bb1879f..4ca2a3e65 100644 --- a/pkg/terraform/executor.go +++ b/pkg/terraform/executor.go @@ -534,10 +534,8 @@ func (ex *Executor) checkVersion() error { return fmt.Errorf("checking Terraform version: %w", err) } - constraints, err := version.NewConstraint(requiredVersion) - if err != nil { - return fmt.Errorf("checking Terraform version: %w", err) - } + // requiredVersion is const, so we test it in unit tests. + constraints, _ := version.NewConstraint(requiredVersion) if !constraints.Check(v) { return fmt.Errorf("version '%s' of Terraform not supported. Needed %s", v, constraints) diff --git a/pkg/terraform/executor_internal_test.go b/pkg/terraform/executor_internal_test.go new file mode 100644 index 000000000..ff09f1336 --- /dev/null +++ b/pkg/terraform/executor_internal_test.go @@ -0,0 +1,27 @@ +// Copyright 2020 The Lokomotive 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 terraform + +import ( + "testing" + + "github.com/hashicorp/go-version" +) + +func TestRequiredVersion(t *testing.T) { + if _, err := version.NewConstraint(requiredVersion); err != nil { + t.Fatalf("requiredVersion const must be valid version constraint, got: %v", err) + } +} From ef514c59199dcddc8ce1371933ea1f9a66b3e5d0 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 28 Sep 2020 15:09:36 +0200 Subject: [PATCH 5/7] pkg/terraform: make error messages from checkVersion more useful Signed-off-by: Mateusz Gozdek --- pkg/terraform/executor.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/terraform/executor.go b/pkg/terraform/executor.go index 4ca2a3e65..5adc781b8 100644 --- a/pkg/terraform/executor.go +++ b/pkg/terraform/executor.go @@ -516,22 +516,23 @@ func (ex *Executor) logPath(id int) string { func (ex *Executor) checkVersion() error { vOutput, err := ex.executeSync("--version") if err != nil { - return fmt.Errorf("checking Terraform version: %w", err) + return fmt.Errorf("executing 'terraform --version': %w", err) } + format := "Terraform v%s\n" var vStr string - n, err := fmt.Sscanf(string(vOutput), "Terraform v%s\n", &vStr) + n, err := fmt.Sscanf(string(vOutput), format, &vStr) if err != nil { - return fmt.Errorf("checking Terraform version: %w", err) + return fmt.Errorf("output %q does not match format %q: %w", string(vOutput), format, err) } if n != 1 { - return fmt.Errorf("error parsing Terraform version") + return fmt.Errorf("version not found in 'terraform --version' output") } v, err := version.NewVersion(vStr) if err != nil { - return fmt.Errorf("checking Terraform version: %w", err) + return fmt.Errorf("parsing Terraform version %q: %w", vStr, err) } // requiredVersion is const, so we test it in unit tests. From 647cc3413eb4050ef2e3d9e0008960ba3f26c937 Mon Sep 17 00:00:00 2001 From: Mateusz Gozdek Date: Mon, 28 Sep 2020 15:10:01 +0200 Subject: [PATCH 6/7] pkg/terraform: add unit tests for checkVersion() Signed-off-by: Mateusz Gozdek --- pkg/terraform/executor_test.go | 100 +++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 pkg/terraform/executor_test.go diff --git a/pkg/terraform/executor_test.go b/pkg/terraform/executor_test.go new file mode 100644 index 000000000..8d5abed75 --- /dev/null +++ b/pkg/terraform/executor_test.go @@ -0,0 +1,100 @@ +// Copyright 2020 The Lokomotive 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 terraform_test + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/kinvolk/lokomotive/pkg/terraform" +) + +//nolint:funlen +func TestVersionConstraint(t *testing.T) { + cases := map[string]struct { + output string + expectError bool + }{ + "valid": { + output: "Terraform v0.12.10", + }, + "outdated": { + output: "Terraform v0.11.0", + expectError: true, + }, + "unsupported": { + output: "Terraform v0.13.5", + expectError: true, + }, + "with extra test": { + output: `Terraform v0.12.11 + +Your version of Terraform is out of date! The latest version +is 0.13.3. You can update by downloading from https://www.terraform.io/downloads.html`, + }, + } + + for n, c := range cases { + c := c + + t.Run(n, func(t *testing.T) { + tmpDir, err := ioutil.TempDir("", "lokoctl-tests-") + if err != nil { + t.Fatalf("Creating tmp dir should succeed, got: %v", err) + } + + t.Cleanup(func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Logf("Removing directory %q: %v", tmpDir, err) + } + }) + + v := []byte(fmt.Sprintf(`#!/bin/sh + cat < Date: Wed, 30 Sep 2020 12:03:54 +0200 Subject: [PATCH 7/7] pkg/terraform: add missing license header to e2e test file Signed-off-by: Mateusz Gozdek --- pkg/terraform/executor_e2e_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/terraform/executor_e2e_test.go b/pkg/terraform/executor_e2e_test.go index 91312d0c5..031be45e3 100644 --- a/pkg/terraform/executor_e2e_test.go +++ b/pkg/terraform/executor_e2e_test.go @@ -1,3 +1,17 @@ +// Copyright 2020 The Lokomotive 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. + //+build e2e package terraform_test