Merge pull request #46176 from vvoland/refactor-integration-tag-subtests

integration/tag: Use subtests, make parallel and move to client unit test

client/image_tag_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/testutil"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -36,15 +37,52 @@ func TestImageTagInvalidReference(t *testing.T) {
 	}
 }
 
+// Ensure we don't allow the use of invalid repository names or tags; these tag operations should fail.
 func TestImageTagInvalidSourceImageName(t *testing.T) {
+	ctx := context.Background()
+
 	client := &Client{
-		client: newMockClient(errorMock(http.StatusInternalServerError, "Server error")),
+		client: newMockClient(errorMock(http.StatusInternalServerError, "client should not have made an API call")),
+	}
+
+	invalidRepos := []string{"fo$z$", "Foo@3cc", "Foo$3", "Foo*3", "Fo^3", "Foo!3", "F)xcz(", "fo%asd", "FOO/bar", "aa/asdf$$^/aa"}
+	for _, repo := range invalidRepos {
+		repo := repo
+		t.Run("invalidRepo/"+repo, func(t *testing.T) {
+			t.Parallel()
+			err := client.ImageTag(ctx, "busybox", repo)
+			assert.Check(t, is.ErrorContains(err, "not a valid repository/tag"))
+		})
 	}
 
-	err := client.ImageTag(context.Background(), "invalid_source_image_name_", "repo:tag")
-	if err == nil || err.Error() != `Error parsing reference: "invalid_source_image_name_" is not a valid repository/tag: invalid reference format` {
-		t.Fatalf("expected Parsing Reference Error, got %v", err)
+	longTag := testutil.GenerateRandomAlphaOnlyString(121)
+	invalidTags := []string{"repo:fo$z$", "repo:Foo@3cc", "repo:Foo$3", "repo:Foo*3", "repo:Fo^3", "repo:Foo!3", "repo:%goodbye", "repo:#hashtagit", "repo:F)xcz(", "repo:-foo", "repo:..", longTag}
+	for _, repotag := range invalidTags {
+		repotag := repotag
+		t.Run("invalidTag/"+repotag, func(t *testing.T) {
+			t.Parallel()
+			err := client.ImageTag(ctx, "busybox", repotag)
+			assert.Check(t, is.ErrorContains(err, "not a valid repository/tag"))
+		})
 	}
+
+	t.Run("test repository name begin with '-'", func(t *testing.T) {
+		t.Parallel()
+		err := client.ImageTag(ctx, "busybox:latest", "-busybox:test")
+		assert.Check(t, is.ErrorContains(err, "Error parsing reference"))
+	})
+
+	t.Run("test namespace name begin with '-'", func(t *testing.T) {
+		t.Parallel()
+		err := client.ImageTag(ctx, "busybox:latest", "-test/busybox:test")
+		assert.Check(t, is.ErrorContains(err, "Error parsing reference"))
+	})
+
+	t.Run("test index name begin with '-'", func(t *testing.T) {
+		t.Parallel()
+		err := client.ImageTag(ctx, "busybox:latest", "-index:5000/busybox:test")
+		assert.Check(t, is.ErrorContains(err, "Error parsing reference"))
+	})
 }
 
 func TestImageTagHexSource(t *testing.T) {

integration/image/tag_test.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/docker/docker/testutil"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -27,43 +26,11 @@ func TestTagUnprefixedRepoByNameOrName(t *testing.T) {
 	assert.NilError(t, err)
 }
 
-// ensure we don't allow the use of invalid repository names or tags; these tag operations should fail
-// TODO (yongtang): Migrate to unit tests
-func TestTagInvalidReference(t *testing.T) {
+func TestTagUsingDigestAlgorithmAsName(t *testing.T) {
 	defer setupTest(t)()
 	client := testEnv.APIClient()
 	ctx := context.Background()
-
-	invalidRepos := []string{"fo$z$", "Foo@3cc", "Foo$3", "Foo*3", "Fo^3", "Foo!3", "F)xcz(", "fo%asd", "FOO/bar"}
-
-	for _, repo := range invalidRepos {
-		err := client.ImageTag(ctx, "busybox", repo)
-		assert.Check(t, is.ErrorContains(err, "not a valid repository/tag"))
-	}
-
-	longTag := testutil.GenerateRandomAlphaOnlyString(121)
-
-	invalidTags := []string{"repo:fo$z$", "repo:Foo@3cc", "repo:Foo$3", "repo:Foo*3", "repo:Fo^3", "repo:Foo!3", "repo:%goodbye", "repo:#hashtagit", "repo:F)xcz(", "repo:-foo", "repo:..", longTag}
-
-	for _, repotag := range invalidTags {
-		err := client.ImageTag(ctx, "busybox", repotag)
-		assert.Check(t, is.ErrorContains(err, "not a valid repository/tag"))
-	}
-
-	// test repository name begin with '-'
-	err := client.ImageTag(ctx, "busybox:latest", "-busybox:test")
-	assert.Check(t, is.ErrorContains(err, "Error parsing reference"))
-
-	// test namespace name begin with '-'
-	err = client.ImageTag(ctx, "busybox:latest", "-test/busybox:test")
-	assert.Check(t, is.ErrorContains(err, "Error parsing reference"))
-
-	// test index name begin with '-'
-	err = client.ImageTag(ctx, "busybox:latest", "-index:5000/busybox:test")
-	assert.Check(t, is.ErrorContains(err, "Error parsing reference"))
-
-	// test setting tag fails
-	err = client.ImageTag(ctx, "busybox:latest", "sha256:sometag")
+	err := client.ImageTag(ctx, "busybox:latest", "sha256:sometag")
 	assert.Check(t, is.ErrorContains(err, "refusing to create an ambiguous tag using digest algorithm as name"))
 }
 
@@ -76,8 +43,12 @@ func TestTagValidPrefixedRepo(t *testing.T) {
 	validRepos := []string{"fooo/bar", "fooaa/test", "foooo:t", "HOSTNAME.DOMAIN.COM:443/foo/bar"}
 
 	for _, repo := range validRepos {
-		err := client.ImageTag(ctx, "busybox", repo)
-		assert.NilError(t, err)
+		repo := repo
+		t.Run(repo, func(t *testing.T) {
+			t.Parallel()
+			err := client.ImageTag(ctx, "busybox", repo)
+			assert.NilError(t, err)
+		})
 	}
 }
 
@@ -107,18 +78,20 @@ func TestTagOfficialNames(t *testing.T) {
 	}
 
 	for _, name := range names {
-		err := client.ImageTag(ctx, "busybox", name+":latest")
-		assert.NilError(t, err)
-
-		// ensure we don't have multiple tag names.
-		insp, _, err := client.ImageInspectWithRaw(ctx, "busybox")
-		assert.NilError(t, err)
-		assert.Assert(t, !is.Contains(insp.RepoTags, name)().Success())
-	}
-
-	for _, name := range names {
-		err := client.ImageTag(ctx, name+":latest", "fooo/bar:latest")
-		assert.NilError(t, err)
+		name := name
+		t.Run("tag from busybox to "+name, func(t *testing.T) {
+			err := client.ImageTag(ctx, "busybox", name+":latest")
+			assert.NilError(t, err)
+
+			// ensure we don't have multiple tag names.
+			insp, _, err := client.ImageInspectWithRaw(ctx, "busybox")
+			assert.NilError(t, err)
+			// TODO(vvoland): Not sure what's actually being tested here. Is is still doing anything useful?
+			assert.Assert(t, !is.Contains(insp.RepoTags, name)().Success())
+
+			err = client.ImageTag(ctx, name+":latest", "test-tag-official-names/foobar:latest")
+			assert.NilError(t, err)
+		})
 	}
 }