From 4972e6b52a8f257c4fd03220fdeb7373b9d786de Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Fri, 22 Jan 2021 10:31:37 -0800 Subject: [PATCH 1/2] refactor download only test for better visibility --- test/integration/aaa_download_only_test.go | 108 +++++++++++++-------- 1 file changed, 65 insertions(+), 43 deletions(-) diff --git a/test/integration/aaa_download_only_test.go b/test/integration/aaa_download_only_test.go index b39d98bae1..0b36b225f8 100644 --- a/test/integration/aaa_download_only_test.go +++ b/test/integration/aaa_download_only_test.go @@ -55,26 +55,31 @@ func TestDownloadOnly(t *testing.T) { constants.NewestKubernetesVersion, } + // Small optimization, don't run the exact same set of tests twice + if constants.DefaultKubernetesVersion == constants.NewestKubernetesVersion { + versions = versions[:len(versions)-1] + } + for _, v := range versions { t.Run(v, func(t *testing.T) { defer PostMortemLogs(t, profile) - // --force to avoid uid check - args := append([]string{"start", "-o=json", "--download-only", "-p", profile, "--force", "--alsologtostderr", fmt.Sprintf("--kubernetes-version=%s", v), fmt.Sprintf("--container-runtime=%s", r)}, StartArgs()...) - - rt, err := Run(t, exec.CommandContext(ctx, Target(), args...)) - if rrr == nil { - // Preserve the initial run-result for debugging - rrr = rt - } - if err != nil { - t.Errorf("failed to download only. args: %q %v", args, err) - } t.Run("check json events", func(t *testing.T) { + // --force to avoid uid check + args := append([]string{"start", "-o=json", "--download-only", "-p", profile, "--force", "--alsologtostderr", fmt.Sprintf("--kubernetes-version=%s", v), fmt.Sprintf("--container-runtime=%s", r)}, StartArgs()...) + rt, err := Run(t, exec.CommandContext(ctx, Target(), args...)) + if rrr == nil { + // Preserve the initial run-result for debugging + rrr = rt + } + if err != nil { + t.Errorf("failed to download only. args: %q %v", args, err) + } + s := bufio.NewScanner(bytes.NewReader(rt.Stdout.Bytes())) for s.Scan() { var rtObj map[string]interface{} - err = json.Unmarshal(s.Bytes(), &rtObj) + err := json.Unmarshal(s.Bytes(), &rtObj) if err != nil { t.Errorf("failed to parse output: %v", err) } else if step, ok := rtObj["data"]; ok { @@ -87,23 +92,36 @@ func TestDownloadOnly(t *testing.T) { } }) - // skip for none, as none driver does not have preload feature. - if !NoneDriver() { + preloadExists := false + t.Run("check preload exists", func(t *testing.T) { + // skip for none, as none driver does not have preload feature. + if NoneDriver() { + t.Skip("None driver does not have preload") + } if download.PreloadExists(v, r, true) { // Just make sure the tarball path exists if _, err := os.Stat(download.TarballPath(v, r)); err != nil { t.Errorf("failed to verify preloaded tarball file exists: %v", err) } - return + preloadExists = true + } else { + t.Skip("No preload image") + } + }) + + t.Run("check cached images", func(t *testing.T) { + // skip verify for cache images if --driver=none + if NoneDriver() { + t.Skip("None driver has no cache") + } + if preloadExists { + t.Skip("Preload exists, images won't be cached") + } + imgs, err := images.Kubeadm("", v) + if err != nil { + t.Errorf("failed to get kubeadm images for %v: %+v", v, err) } - } - imgs, err := images.Kubeadm("", v) - if err != nil { - t.Errorf("failed to get kubeadm images for %v: %+v", v, err) - } - // skip verify for cache images if --driver=none - if !NoneDriver() { for _, img := range imgs { img = strings.Replace(img, ":", "_", 1) // for example kube-scheduler:v1.15.2 --> kube-scheduler_v1.15.2 fp := filepath.Join(localpath.MiniPath(), "cache", "images", img) @@ -112,30 +130,34 @@ func TestDownloadOnly(t *testing.T) { t.Errorf("expected image file exist at %q but got error: %v", fp, err) } } - } + }) - // checking binaries downloaded (kubelet,kubeadm) - for _, bin := range constants.KubernetesReleaseBinaries { - fp := filepath.Join(localpath.MiniPath(), "cache", "linux", v, bin) - _, err := os.Stat(fp) - if err != nil { + t.Run("check binaries", func(t *testing.T) { + // checking binaries downloaded (kubelet,kubeadm) + for _, bin := range constants.KubernetesReleaseBinaries { + fp := filepath.Join(localpath.MiniPath(), "cache", "linux", v, bin) + _, err := os.Stat(fp) + if err != nil { + t.Errorf("expected the file for binary exist at %q but got error %v", fp, err) + } + } + }) + + t.Run("check kubectl", func(t *testing.T) { + // If we are on darwin/windows, check to make sure OS specific kubectl has been downloaded + // as well for the `minikube kubectl` command + if runtime.GOOS == "linux" { + t.Skip("Test for darwin and windows") + } + binary := "kubectl" + if runtime.GOOS == "windows" { + binary = "kubectl.exe" + } + fp := filepath.Join(localpath.MiniPath(), "cache", runtime.GOOS, v, binary) + if _, err := os.Stat(fp); err != nil { t.Errorf("expected the file for binary exist at %q but got error %v", fp, err) } - } - - // If we are on darwin/windows, check to make sure OS specific kubectl has been downloaded - // as well for the `minikube kubectl` command - if runtime.GOOS == "linux" { - return - } - binary := "kubectl" - if runtime.GOOS == "windows" { - binary = "kubectl.exe" - } - fp := filepath.Join(localpath.MiniPath(), "cache", runtime.GOOS, v, binary) - if _, err := os.Stat(fp); err != nil { - t.Errorf("expected the file for binary exist at %q but got error %v", fp, err) - } + }) }) } From b694e0e6273f72c6194c5f47bd810dca49f6ee56 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Mon, 25 Jan 2021 14:37:35 -0800 Subject: [PATCH 2/2] shorten subtest names --- test/integration/aaa_download_only_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/aaa_download_only_test.go b/test/integration/aaa_download_only_test.go index 1b59c7136f..48610a9dd3 100644 --- a/test/integration/aaa_download_only_test.go +++ b/test/integration/aaa_download_only_test.go @@ -62,7 +62,7 @@ func TestDownloadOnly(t *testing.T) { t.Run(v, func(t *testing.T) { defer PostMortemLogs(t, profile) - t.Run("check json events", func(t *testing.T) { + t.Run("json-events", func(t *testing.T) { // --force to avoid uid check args := append([]string{"start", "-o=json", "--download-only", "-p", profile, "--force", "--alsologtostderr", fmt.Sprintf("--kubernetes-version=%s", v), fmt.Sprintf("--container-runtime=%s", containerRuntime)}, StartArgs()...) rt, err := Run(t, exec.CommandContext(ctx, Target(), args...)) @@ -91,7 +91,7 @@ func TestDownloadOnly(t *testing.T) { }) preloadExists := false - t.Run("check preload exists", func(t *testing.T) { + t.Run("preload-exists", func(t *testing.T) { // skip for none, as none driver does not have preload feature. if NoneDriver() { t.Skip("None driver does not have preload") @@ -107,7 +107,7 @@ func TestDownloadOnly(t *testing.T) { } }) - t.Run("check cached images", func(t *testing.T) { + t.Run("cached-images", func(t *testing.T) { // skip verify for cache images if --driver=none if NoneDriver() { t.Skip("None driver has no cache") @@ -130,7 +130,7 @@ func TestDownloadOnly(t *testing.T) { } }) - t.Run("check binaries", func(t *testing.T) { + t.Run("binaries", func(t *testing.T) { // checking binaries downloaded (kubelet,kubeadm) for _, bin := range constants.KubernetesReleaseBinaries { fp := filepath.Join(localpath.MiniPath(), "cache", "linux", v, bin) @@ -141,7 +141,7 @@ func TestDownloadOnly(t *testing.T) { } }) - t.Run("check kubectl", func(t *testing.T) { + t.Run("kubectl", func(t *testing.T) { // If we are on darwin/windows, check to make sure OS specific kubectl has been downloaded // as well for the `minikube kubectl` command if runtime.GOOS == "linux" {