Increase the tests coverage for import-image command

Author: soltysh

pkg/cmd/cli/cmd/importimage_test.go

@@ -13,16 +13,41 @@ import (
 )
 
 func TestCreateImageImport(t *testing.T) {
-	testCases := []struct {
+	testCases := map[string]struct {
 		name     string
 		stream   *imageapi.ImageStream
 		all      bool
+		confirm  bool
 		insecure *bool
 		err      string
 		expected []imageapi.ImageImportSpec
 	}{
-		{
-			// 0: checking import's from when only .spec.dockerImageRepository is set, no status
+		"import from non-existing": {
+			name: "nonexisting",
+			err:  "pass --confirm to create and import",
+		},
+		"confirmed import from non-existing": {
+			name:    "nonexisting",
+			confirm: true,
+			expected: []imageapi.ImageImportSpec{{
+				From: kapi.ObjectReference{
+					Kind: "DockerImage",
+					Name: "nonexisting",
+				},
+			}},
+		},
+		"confirmed import all from non-existing": {
+			name:    "nonexisting",
+			all:     true,
+			confirm: true,
+			expected: []imageapi.ImageImportSpec{{
+				From: kapi.ObjectReference{
+					Kind: "DockerImage",
+					Name: "nonexisting",
+				},
+			}},
+		},
+		"import from .spec.dockerImageRepository": {
 			name: "testis",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -41,8 +66,7 @@ func TestCreateImageImport(t *testing.T) {
 				},
 			}},
 		},
-		{
-			// 1: checking import's from when only .spec.dockerImageRepository is set, no status (with all flag set)
+		"import all from .spec.dockerImageRepository": {
 			name: "testis",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -62,8 +86,7 @@ func TestCreateImageImport(t *testing.T) {
 				},
 			}},
 		},
-		{
-			// 2: with --all flag only .spec.dockerImageRepository is handled
+		"import all error for .spec.dockerImageRepository": {
 			name: "testis",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -81,8 +104,7 @@ func TestCreateImageImport(t *testing.T) {
 			all: true,
 			err: "all is applicable only to images with spec.dockerImageRepository",
 		},
-		{
-			// 3: empty image stream
+		"empty image stream": {
 			name: "testis",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -92,8 +114,7 @@ func TestCreateImageImport(t *testing.T) {
 			},
 			err: "image stream has not defined anything to import",
 		},
-		{
-			// 4: correct import of latest tag with tags specified in .spec.Tags
+		"import latest tag": {
 			name: "testis:latest",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -115,8 +136,29 @@ func TestCreateImageImport(t *testing.T) {
 				},
 			}},
 		},
-		{
-			// 5: import latest from image stream which has only tags specified and no latest
+		"import existing tag": {
+			name: "testis:existing",
+			stream: &imageapi.ImageStream{
+				ObjectMeta: kapi.ObjectMeta{
+					Name:      "testis",
+					Namespace: "other",
+				},
+				Spec: imageapi.ImageStreamSpec{
+					Tags: map[string]imageapi.TagReference{
+						"existing": {
+							From: &kapi.ObjectReference{Kind: "DockerImage", Name: "repo.com/somens/someimage:latest"},
+						},
+					},
+				},
+			},
+			expected: []imageapi.ImageImportSpec{{
+				From: kapi.ObjectReference{
+					Kind: "DockerImage",
+					Name: "repo.com/somens/someimage:latest",
+				},
+			}},
+		},
+		"import non-existing tag": {
 			name: "testis:latest",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -133,8 +175,7 @@ func TestCreateImageImport(t *testing.T) {
 			},
 			err: "does not exist on the image stream",
 		},
-		{
-			// 6: insecure annotation should be applied to tags if exists
+		"use insecure annotation": {
 			name: "testis",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -155,8 +196,7 @@ func TestCreateImageImport(t *testing.T) {
 				ImportPolicy: imageapi.TagImportPolicy{Insecure: true},
 			}},
 		},
-		{
-			// 7: insecure annotation should be overridden by the flag
+		"insecure flag overrides insecure annotation": {
 			name: "testis",
 			stream: &imageapi.ImageStream{
 				ObjectMeta: kapi.ObjectMeta{
@@ -180,52 +220,57 @@ func TestCreateImageImport(t *testing.T) {
 		},
 	}
 
-	for idx, test := range testCases {
-		fake := testclient.NewSimpleFake(test.stream)
+	for name, test := range testCases {
+		var fake *testclient.Fake
+		if test.stream == nil {
+			fake = testclient.NewSimpleFake()
+		} else {
+			fake = testclient.NewSimpleFake(test.stream)
+		}
 		o := ImportImageOptions{
-			Target:    test.stream.Name,
-			All:       test.all,
-			Insecure:  test.insecure,
-			Namespace: test.stream.Namespace,
-			isClient:  fake.ImageStreams(test.stream.Namespace),
+			Target:   test.name,
+			All:      test.all,
+			Insecure: test.insecure,
+			Confirm:  test.confirm,
+			isClient: fake.ImageStreams(""),
 		}
 		// we need to run Validate, because it sets appropriate Name and Tag
 		if err := o.Validate(&cobra.Command{}); err != nil {
-			t.Errorf("(%d) unexpected error: %v", idx, err)
+			t.Errorf("%s: unexpected error: %v", name, err)
 		}
 
 		_, isi, err := o.createImageImport()
 		// check errors
 		if len(test.err) > 0 {
 			if err == nil || !strings.Contains(err.Error(), test.err) {
-				t.Errorf("(%d) unexpected error: expected %s, got %v", idx, test.err, err)
+				t.Errorf("%s: unexpected error: expected %s, got %v", name, test.err, err)
 			}
 			if isi != nil {
-				t.Errorf("(%d) unexpected import spec: expected nil, got %#v", idx, isi)
+				t.Errorf("%s: unexpected import spec: expected nil, got %#v", name, isi)
 			}
 			continue
 		}
 		if len(test.err) == 0 && err != nil {
-			t.Errorf("(%d) unexpected error: %v", idx, err)
+			t.Errorf("%s: unexpected error: %v", name, err)
 			continue
 		}
 		// check values
 		if test.all {
 			if !kapi.Semantic.DeepEqual(isi.Spec.Repository.From, test.expected[0].From) {
-				t.Errorf("(%d) unexpected import spec, expected %#v, got %#v", idx, test.expected[0].From, isi.Spec.Repository.From)
+				t.Errorf("%s: unexpected import spec.From, expected %#v, got %#v", name, test.expected[0].From, isi.Spec.Repository.From)
 			}
 		} else {
 			if len(isi.Spec.Images) != len(test.expected) {
-				t.Errorf("(%d) unexpected number of images, expected %d, got %d", idx, len(test.expected), len(isi.Spec.Images))
+				t.Errorf("%s: unexpected number of images, expected %d, got %d", name, len(test.expected), len(isi.Spec.Images))
 			}
 			for i := 0; i < len(test.expected); i++ {
 				actual := isi.Spec.Images[i]
 				expected := test.expected[i]
 				if !kapi.Semantic.DeepEqual(actual.ImportPolicy, expected.ImportPolicy) {
-					t.Errorf("(%d) unexpected import[%d] policy, expected %v, got %v", idx, i, expected.ImportPolicy, actual.ImportPolicy)
+					t.Errorf("%s: unexpected import[%d] policy, expected %v, got %v", name, i, expected.ImportPolicy, actual.ImportPolicy)
 				}
 				if !kapi.Semantic.DeepEqual(actual.From, expected.From) {
-					t.Errorf("(%d) unexpected import[%d] from, expected %#v, got %#v", idx, i, expected.From, actual.From)
+					t.Errorf("%s: unexpected import[%d] from, expected %#v, got %#v", name, i, expected.From, actual.From)
 				}
 			}
 		}