diff --git a/cmd/kops/BUILD.bazel b/cmd/kops/BUILD.bazel index 2e1ba1df3e..365b7c57ac 100644 --- a/cmd/kops/BUILD.bazel +++ b/cmd/kops/BUILD.bazel @@ -179,6 +179,5 @@ go_test( "//vendor/golang.org/x/crypto/ssh:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", - "//vendor/k8s.io/klog:go_default_library", ], ) diff --git a/cmd/kops/create_cluster_integration_test.go b/cmd/kops/create_cluster_integration_test.go index 21a92e04d9..ced969ba46 100644 --- a/cmd/kops/create_cluster_integration_test.go +++ b/cmd/kops/create_cluster_integration_test.go @@ -19,7 +19,6 @@ package main import ( "bytes" "io/ioutil" - "os" "path" "strings" "testing" @@ -27,14 +26,12 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" - "k8s.io/klog" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/kops/cloudmock/aws/mockec2" "k8s.io/kops/cmd/kops/util" "k8s.io/kops/pkg/apis/kops" - "k8s.io/kops/pkg/diff" "k8s.io/kops/pkg/kopscodecs" "k8s.io/kops/pkg/testutils" "k8s.io/kops/upup/pkg/fi" @@ -245,37 +242,6 @@ func runCreateClusterIntegrationTest(t *testing.T, srcDir string, version string yamlAll = append(yamlAll, actualYAML) } - expectedYAMLBytes, err := ioutil.ReadFile(path.Join(srcDir, expectedClusterPath)) - if err != nil { - t.Fatalf("unexpected error reading expected YAML: %v", err) - } - - //on windows, with git set to autocrlf, the reference files on disk have windows line endings - expectedYAML := strings.Replace(strings.TrimSpace(string(expectedYAMLBytes)), "\r\n", "\n", -1) - actualYAML := strings.Join(yamlAll, "\n\n---\n\n") - if actualYAML != expectedYAML { - p := path.Join(srcDir, expectedClusterPath) - - if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { - t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", p) - - // Format nicely - keep git happy - s := actualYAML - s = strings.TrimSpace(s) - s = s + "\n" - - if err := ioutil.WriteFile(p, []byte(s), 0644); err != nil { - t.Errorf("error writing expected output %s: %v", p, err) - } - } - - klog.Infof("Actual YAML:\n%s\n", actualYAML) - - diffString := diff.FormatDiff(expectedYAML, actualYAML) - t.Logf("diff:\n%s\n", diffString) - - t.Errorf("YAML differed from expected (%s)", p) - } - + testutils.AssertMatchesFile(t, actualYAML, path.Join(srcDir, expectedClusterPath)) } diff --git a/cmd/kops/integration_test.go b/cmd/kops/integration_test.go index bd0a532d84..980287265a 100644 --- a/cmd/kops/integration_test.go +++ b/cmd/kops/integration_test.go @@ -307,27 +307,8 @@ func runTest(t *testing.T, h *testutils.IntegrationTestHarness, clusterName stri if err != nil { t.Fatalf("unexpected error reading actual terraform output: %v", err) } - expectedTF, err := ioutil.ReadFile(path.Join(srcDir, testDataTFPath)) - if err != nil { - t.Fatalf("unexpected error reading expected terraform output: %v", err) - } - expectedTF = bytes.Replace(expectedTF, []byte("\r\n"), []byte("\n"), -1) - if !bytes.Equal(actualTF, expectedTF) { - diffString := diff.FormatDiff(string(expectedTF), string(actualTF)) - t.Logf("diff:\n%s\n", diffString) - - if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { - fp := path.Join(srcDir, testDataTFPath) - t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", fp) - if err := ioutil.WriteFile(fp, actualTF, 0644); err != nil { - t.Errorf("error writing terraform output: %v", err) - } - t.Errorf("terraform output differed from expected") - return // Avoid Fatalf as we want to keep going and update all files - } - t.Fatalf("terraform output differed from expected") - } + testutils.AssertMatchesFile(t, string(actualTF), path.Join(srcDir, testDataTFPath)) } // Compare data files if they are provided @@ -587,10 +568,6 @@ func runTestCloudformation(t *testing.T, clusterName string, srcDir string, vers if err != nil { t.Fatalf("unexpected error reading actual cloudformation output: %v", err) } - expectedCF, err := ioutil.ReadFile(path.Join(srcDir, expectedCfPath)) - if err != nil { - t.Fatalf("unexpected error reading expected cloudformation output: %v", err) - } // Expand out the UserData base64 blob, as otherwise testing is painful extracted := make(map[string]string) @@ -625,28 +602,7 @@ func runTestCloudformation(t *testing.T, clusterName string, srcDir string, vers } actualCF = buf.Bytes() - expectedCFTrimmed := strings.Replace(strings.TrimSpace(string(expectedCF)), "\r\n", "\n", -1) - actualCFTrimmed := strings.TrimSpace(string(actualCF)) - if actualCFTrimmed != expectedCFTrimmed { - diffString := diff.FormatDiff(expectedCFTrimmed, actualCFTrimmed) - t.Logf("diff:\n%s\n", diffString) - - if os.Getenv("KEEP_TEMP_DIR") == "" { - t.Logf("(hint: setting KEEP_TEMP_DIR will preserve test output") - } else { - t.Logf("actual terraform output in %s", actualPath) - } - - if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { - fp := path.Join(srcDir, expectedCfPath) - t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", fp) - if err := ioutil.WriteFile(fp, actualCF, 0644); err != nil { - t.Errorf("error writing expected output file %q: %v", fp, err) - } - } - - t.Fatalf("cloudformation output differed from expected. Test file: %s", path.Join(srcDir, expectedCfPath)) - } + testutils.AssertMatchesFile(t, string(actualCF), path.Join(srcDir, expectedCfPath)) fp := path.Join(srcDir, expectedCfPath+".extracted.yaml") expectedExtracted, err := ioutil.ReadFile(fp) @@ -685,24 +641,7 @@ func runTestCloudformation(t *testing.T, clusterName string, srcDir string, vers } } - if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { - t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", fp) - - // We replace the \r characters so that the yaml output (should) be block-quoted - // Literal quoting is sadly unreadable... - for k, v := range actual { - actual[k] = strings.Replace(v, "\r", "", -1) - } - - b, err := yaml.Marshal(actual) - if err != nil { - t.Errorf("error serializing cloudformation output: %v", err) - } - if err := ioutil.WriteFile(fp, b, 0644); err != nil { - t.Errorf("error writing cloudformation output: %v", err) - } - } - + testutils.AssertMatchesFile(t, string(actualCF), path.Join(srcDir, expectedCfPath)) } } diff --git a/hack/update-expected.sh b/hack/update-expected.sh index 8711cfa5ae..61e5883b1f 100755 --- a/hack/update-expected.sh +++ b/hack/update-expected.sh @@ -20,4 +20,4 @@ set -o pipefail KOPS_ROOT=$(git rev-parse --show-toplevel) cd ${KOPS_ROOT} -UPDATE_CHANNEL_BUILDER_TEST_FIXTURES=true HACK_UPDATE_EXPECTED_IN_PLACE=1 go test ./... +HACK_UPDATE_EXPECTED_IN_PLACE=1 go test ./... diff --git a/nodeup/pkg/model/tests/kubelet/featuregates/tasks.yaml b/nodeup/pkg/model/tests/kubelet/featuregates/tasks.yaml index 1c4642a67a..ba7678d0fc 100644 --- a/nodeup/pkg/model/tests/kubelet/featuregates/tasks.yaml +++ b/nodeup/pkg/model/tests/kubelet/featuregates/tasks.yaml @@ -28,4 +28,4 @@ definition: | enabled: true manageState: true running: true -smartRestart: true \ No newline at end of file +smartRestart: true diff --git a/pkg/model/BUILD.bazel b/pkg/model/BUILD.bazel index 01df3d9325..9274fdcbc4 100644 --- a/pkg/model/BUILD.bazel +++ b/pkg/model/BUILD.bazel @@ -66,7 +66,7 @@ go_test( deps = [ "//pkg/apis/kops:go_default_library", "//pkg/apis/nodeup:go_default_library", - "//pkg/diff:go_default_library", + "//pkg/testutils:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", ], ) diff --git a/pkg/model/bootstrapscript_test.go b/pkg/model/bootstrapscript_test.go index 274f560aef..64fa304979 100644 --- a/pkg/model/bootstrapscript_test.go +++ b/pkg/model/bootstrapscript_test.go @@ -17,14 +17,12 @@ limitations under the License. package model import ( - "io/ioutil" - "os" "strings" "testing" "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/apis/nodeup" - "k8s.io/kops/pkg/diff" + "k8s.io/kops/pkg/testutils" ) func Test_ProxyFunc(t *testing.T) { @@ -143,23 +141,7 @@ func TestBootstrapUserData(t *testing.T) { continue } - expectedBytes, err := ioutil.ReadFile(x.ExpectedFilePath) - if err != nil { - t.Fatalf("unexpected error reading ExpectedFilePath %q: %v", x.ExpectedFilePath, err) - } - - if actual != string(expectedBytes) { - if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { - t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", x.ExpectedFilePath) - if err := ioutil.WriteFile(x.ExpectedFilePath, []byte(actual), 0644); err != nil { - t.Errorf("error writing expected output: %v", err) - } - } - - diffString := diff.FormatDiff(string(expectedBytes), actual) - t.Errorf("case %d failed, actual output differed from expected (%s).", i, x.ExpectedFilePath) - t.Logf("diff:\n%s\n", diffString) - } + testutils.AssertMatchesFile(t, actual, x.ExpectedFilePath) } } diff --git a/pkg/testutils/modelharness.go b/pkg/testutils/modelharness.go index 5cf4a67988..1631a4bcb0 100644 --- a/pkg/testutils/modelharness.go +++ b/pkg/testutils/modelharness.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path" + "path/filepath" "sort" "strings" "testing" @@ -95,27 +96,58 @@ func ValidateTasks(t *testing.T, basedir string, context *fi.ModelBuilderContext } actualTasksYaml := strings.Join(yamls, "\n---\n") + actualTasksYaml = strings.TrimSpace(actualTasksYaml) tasksYamlPath := path.Join(basedir, "tasks.yaml") - expectedTasksYamlBytes, err := ioutil.ReadFile(tasksYamlPath) + + AssertMatchesFile(t, actualTasksYaml, tasksYamlPath) +} + +// AssertMatchesFile matches the actual value to a with expected file. +// If HACK_UPDATE_EXPECTED_IN_PLACE is set, it will write the actual value to the expected file, +// which is very handy when updating our tests. +func AssertMatchesFile(t *testing.T, actual string, p string) { + actual = strings.TrimSpace(actual) + + expectedBytes, err := ioutil.ReadFile(p) if err != nil { - t.Fatalf("error reading file %q: %v", tasksYamlPath, err) + t.Fatalf("error reading file %q: %v", p, err) + } + expected := strings.TrimSpace(string(expectedBytes)) + + //on windows, with git set to autocrlf, the reference files on disk have windows line endings + expected = strings.Replace(expected, "\r\n", "\n", -1) + + if actual == expected { + return } - actualTasksYaml = strings.TrimSpace(actualTasksYaml) - expectedTasksYaml := strings.TrimSpace(string(expectedTasksYamlBytes)) + if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { + t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", p) - if expectedTasksYaml != actualTasksYaml { - if os.Getenv("HACK_UPDATE_EXPECTED_IN_PLACE") != "" { - t.Logf("HACK_UPDATE_EXPECTED_IN_PLACE: writing expected output %s", tasksYamlPath) - if err := ioutil.WriteFile(tasksYamlPath, []byte(actualTasksYaml), 0644); err != nil { - t.Errorf("error writing expected output %s: %v", tasksYamlPath, err) - } + // Keep git happy with a trailing newline + actual += "\n" + + if err := ioutil.WriteFile(p, []byte(actual), 0644); err != nil { + t.Errorf("error writing expected output %s: %v", p, err) } - diffString := diff.FormatDiff(expectedTasksYaml, actualTasksYaml) - t.Logf("diff:\n%s\n", diffString) - - t.Fatalf("tasks differed from expected for test %q", basedir) + // Keep going so we write all files in a test + t.Errorf("output did not match expected for %q", p) + return } + + diffString := diff.FormatDiff(expected, actual) + t.Logf("diff:\n%s\n", diffString) + + abs, err := filepath.Abs(p) + if err != nil { + t.Errorf("unable to get absolute path for %q: %v", p, err) + } else { + p = abs + } + + t.Logf("to update golden output automatically, run hack/update-expected.sh") + + t.Fatalf("output did not match expected for %q", p) } diff --git a/tests/integration/update_cluster/additional_user-data/cloudformation.json b/tests/integration/update_cluster/additional_user-data/cloudformation.json index 08d8b7f9d7..3bdb45e01b 100644 --- a/tests/integration/update_cluster/additional_user-data/cloudformation.json +++ b/tests/integration/update_cluster/additional_user-data/cloudformation.json @@ -865,4 +865,4 @@ } } } -} \ No newline at end of file +} diff --git a/tests/integration/update_cluster/mixed_instances/cloudformation.json b/tests/integration/update_cluster/mixed_instances/cloudformation.json index 9f0d67aafe..9ee86f98c1 100644 --- a/tests/integration/update_cluster/mixed_instances/cloudformation.json +++ b/tests/integration/update_cluster/mixed_instances/cloudformation.json @@ -1240,4 +1240,4 @@ } } } -} \ No newline at end of file +} diff --git a/tests/integration/update_cluster/mixed_instances_spot/cloudformation.json b/tests/integration/update_cluster/mixed_instances_spot/cloudformation.json index c19e0209c3..65ef4fec38 100644 --- a/tests/integration/update_cluster/mixed_instances_spot/cloudformation.json +++ b/tests/integration/update_cluster/mixed_instances_spot/cloudformation.json @@ -1241,4 +1241,4 @@ } } } -} \ No newline at end of file +} diff --git a/upup/pkg/fi/cloudup/BUILD.bazel b/upup/pkg/fi/cloudup/BUILD.bazel index 5de627dab9..cbe72fec70 100644 --- a/upup/pkg/fi/cloudup/BUILD.bazel +++ b/upup/pkg/fi/cloudup/BUILD.bazel @@ -111,7 +111,6 @@ go_test( "//pkg/apis/kops/validation:go_default_library", "//pkg/assets:go_default_library", "//pkg/client/simple/vfsclientset:go_default_library", - "//pkg/diff:go_default_library", "//pkg/kopscodecs:go_default_library", "//pkg/model:go_default_library", "//pkg/templates:go_default_library", diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go index 90253212fe..22e82c5cbf 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go +++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go @@ -18,15 +18,12 @@ package cloudup import ( "io/ioutil" - "os" "path" - "strings" "testing" api "k8s.io/kops/pkg/apis/kops" "k8s.io/kops/pkg/assets" "k8s.io/kops/pkg/client/simple/vfsclientset" - "k8s.io/kops/pkg/diff" "k8s.io/kops/pkg/kopscodecs" "k8s.io/kops/pkg/model" "k8s.io/kops/pkg/templates" @@ -121,19 +118,6 @@ func runChannelBuilderTest(t *testing.T, key string) { } expectedManifestPath := path.Join(basedir, "manifest.yaml") - expectedManifest, err := ioutil.ReadFile(expectedManifestPath) - if err != nil { - t.Fatalf("error reading file %q: %v", expectedManifestPath, err) - } - if strings.TrimSpace(string(expectedManifest)) != strings.TrimSpace(actualManifest) { - diffString := diff.FormatDiff(string(expectedManifest), actualManifest) - t.Logf("diff:\n%s\n", diffString) - t.Errorf("manifest differed from expected for test %q", key) - if os.Getenv("UPDATE_CHANNEL_BUILDER_TEST_FIXTURES") == "true" { - ioutil.WriteFile(expectedManifestPath, []byte(actualManifest), 0755) - } else { - t.Logf("to update fixtures automatically, run with UPDATE_CHANNEL_BUILDER_TEST_FIXTURES=true") - } - } + testutils.AssertMatchesFile(t, actualManifest, expectedManifestPath) }