refactor: Cleanup/simplify testcases in /pkg/cli (#1103)

Signed-off-by: Nicol Draghici <idraghic@cisco.com>

pkg/cli/client_elevated_test.go

@@ -5,7 +5,6 @@ package cli //nolint:testpackage
 
 import (
 	"bytes"
-	"context"
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
@@ -13,7 +12,6 @@ import (
 	"os/exec"
 	"path/filepath"
 	"testing"
-	"time"
 
 	. "github.com/smartystreets/goconvey/convey"
 	"gopkg.in/resty.v1"
@@ -21,6 +19,7 @@ import (
 	"zotregistry.io/zot/pkg/api"
 	"zotregistry.io/zot/pkg/api/config"
 	"zotregistry.io/zot/pkg/api/constants"
+	"zotregistry.io/zot/pkg/test"
 )
 
 func TestElevatedPrivilegesTLSNewControllerPrivilegedCert(t *testing.T) {
@@ -84,26 +83,9 @@ func TestElevatedPrivilegesTLSNewControllerPrivilegedCert(t *testing.T) {
 
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = t.TempDir()
-		go func() {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := ctlr.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}()
-
-		// wait till ready
-		for {
-			_, err := resty.R().Get(BaseURL2)
-			if err == nil {
-				break
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-
-		defer func() {
-			ctx := context.Background()
-			_ = ctlr.Server.Shutdown(ctx)
-		}()
 
 		Convey("Certs in privileged path", func() {
 			configPath := makeConfigFile(

pkg/cli/client_test.go

@@ -5,14 +5,12 @@ package cli //nolint:testpackage
 
 import (
 	"bytes"
-	"context"
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
 	"os"
 	"path/filepath"
 	"testing"
-	"time"
 
 	. "github.com/smartystreets/goconvey/convey"
 	"gopkg.in/resty.v1"
@@ -77,26 +75,9 @@ func TestTLSWithAuth(t *testing.T) {
 
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = t.TempDir()
-		go func() {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := ctlr.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}()
-
-		// wait till ready
-		for {
-			_, err := resty.R().Get(BaseSecureURL1)
-			if err == nil {
-				break
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-
-		defer func() {
-			ctx := context.Background()
-			_ = ctlr.Server.Shutdown(ctx)
-		}()
 
 		Convey("Test with htpassw auth", func() {
 			configPath := makeConfigFile(`{"configs":[{"_name":"imagetest","showspinner":false}]}`)
@@ -174,26 +155,9 @@ func TestTLSWithoutAuth(t *testing.T) {
 
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = t.TempDir()
-		go func() {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := ctlr.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}()
-
-		// wait till ready
-		for {
-			_, err := resty.R().Get(BaseURL1)
-			if err == nil {
-				break
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-
-		defer func() {
-			ctx := context.Background()
-			_ = ctlr.Server.Shutdown(ctx)
-		}()
 
 		Convey("Certs in user's home", func() {
 			configPath := makeConfigFile(
@@ -239,26 +203,9 @@ func TestTLSBadCerts(t *testing.T) {
 
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = t.TempDir()
-		go func() {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := ctlr.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}()
-
-		// wait till ready
-		for {
-			_, err := resty.R().Get(BaseURL3)
-			if err == nil {
-				break
-			}
-			time.Sleep(100 * time.Millisecond)
-		}
-
-		defer func() {
-			ctx := context.Background()
-			_ = ctlr.Server.Shutdown(ctx)
-		}()
 
 		Convey("Test with system certs", func() {
 			configPath := makeConfigFile(

pkg/cli/image_cmd_test.go

@@ -313,73 +313,26 @@ func TestSignature(t *testing.T) {
 		}
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = currentDir
-		go func(controller *api.Controller) {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := controller.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}(ctlr)
-		// wait till ready
-		for {
-			_, err := resty.R().Get(url)
-			if err == nil {
-				break
-			}
 
-			time.Sleep(100 * time.Millisecond)
+		cfg, layers, manifest, err := test.GetImageComponents(1)
-		}
-		defer func(controller *api.Controller) {
-			ctx := context.Background()
-			_ = controller.Server.Shutdown(ctx)
-		}(ctlr)
-
-		// create a blob/layer
-		resp, _ := resty.R().Post(url + "/v2/repo7/blobs/uploads/")
-		loc := test.Location(url, resp)
-
-		content := []byte("this is a blob5")
-		digest := godigest.FromBytes(content)
-		_, _ = resty.R().SetQueryParam("digest", digest.String()).
-			SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc)
-
-		// upload image config blob
-		resp, _ = resty.R().Post(url + "/v2/repo7/blobs/uploads/")
-		loc = test.Location(url, resp)
-		cblob, cdigest := test.GetImageConfig()
-
-		_, _ = resty.R().
-			SetContentLength(true).
-			SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))).
-			SetHeader("Content-Type", "application/octet-stream").
-			SetQueryParam("digest", cdigest.String()).
-			SetBody(cblob).
-			Put(loc)
-
-		// create a manifest
-		manifest := ispec.Manifest{
-			Config: ispec.Descriptor{
-				MediaType: "application/vnd.oci.image.config.v1+json",
-				Digest:    cdigest,
-				Size:      int64(len(cblob)),
-			},
-			Layers: []ispec.Descriptor{
-				{
-					MediaType: "application/vnd.oci.image.layer.v1.tar",
-					Digest:    digest,
-					Size:      int64(len(content)),
-				},
-			},
-		}
-		manifest.SchemaVersion = 2
-
-		content, err = json.Marshal(manifest)
 		So(err, ShouldBeNil)
 
-		_, _ = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json").
+		repoName := "repo7"
-			SetBody(content).Put(url + "/v2/repo7/manifests/test:1.0")
+		err = test.UploadImage(
+			test.Image{
+				Config:   cfg,
+				Layers:   layers,
+				Manifest: manifest,
+				Tag:      "test:1.0",
+			}, url, repoName)
+		So(err, ShouldBeNil)
 
-		//	content = []byte("this is a blob5")
+		content, err := json.Marshal(manifest)
-		digest = godigest.FromBytes(content)
+		So(err, ShouldBeNil)
+		digest := godigest.FromBytes(content)
 
 		// generate a keypair
 		if _, err := os.Stat(path.Join(currentDir, "cosign.key")); err != nil {
@@ -415,7 +368,7 @@ func TestSignature(t *testing.T) {
 		str := space.ReplaceAllString(buff.String(), " ")
 		actual := strings.TrimSpace(str)
 		So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE")
-		So(actual, ShouldContainSubstring, "repo7 test:1.0 883fc0c5 true 15B")
+		So(actual, ShouldContainSubstring, "repo7 test:1.0 6742241d true 1B")
 
 		t.Log("Test getting all images using rest calls to get catalog and individual manifests")
 		cmd = MockNewImageCommand(new(searchService))
@@ -428,7 +381,7 @@ func TestSignature(t *testing.T) {
 		str = space.ReplaceAllString(buff.String(), " ")
 		actual = strings.TrimSpace(str)
 		So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE")
-		So(actual, ShouldContainSubstring, "repo7 test:1.0 883fc0c5 true 492B")
+		So(actual, ShouldContainSubstring, "repo7 test:1.0 6742241d true 447B")
 
 		err = os.Chdir(currentWorkingDir)
 		So(err, ShouldBeNil)
@@ -452,70 +405,27 @@ func TestSignature(t *testing.T) {
 		}
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = currentDir
-		go func(controller *api.Controller) {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := controller.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}(ctlr)
-		// wait till ready
-		for {
-			_, err := resty.R().Get(url)
-			if err == nil {
-				break
-			}
 
-			time.Sleep(100 * time.Millisecond)
+		cfg, layers, manifest, err := test.GetImageComponents(1)
-		}
-		defer func(controller *api.Controller) {
-			ctx := context.Background()
-			_ = controller.Server.Shutdown(ctx)
-		}(ctlr)
-
-		// create a blob/layer
-		resp, _ := resty.R().Post(url + "/v2/repo7/blobs/uploads/")
-		loc := test.Location(url, resp)
-
-		content := []byte("this is a blob5")
-		digest := godigest.FromBytes(content)
-		_, _ = resty.R().SetQueryParam("digest", digest.String()).
-			SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc)
-
-		// upload image config blob
-		resp, _ = resty.R().Post(url + "/v2/repo7/blobs/uploads/")
-		loc = test.Location(url, resp)
-		cblob, cdigest := test.GetImageConfig()
-
-		_, _ = resty.R().
-			SetContentLength(true).
-			SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))).
-			SetHeader("Content-Type", "application/octet-stream").
-			SetQueryParam("digest", cdigest.String()).
-			SetBody(cblob).
-			Put(loc)
-
-		// create a manifest
-		manifest := ispec.Manifest{
-			Config: ispec.Descriptor{
-				MediaType: "application/vnd.oci.image.config.v1+json",
-				Digest:    cdigest,
-				Size:      int64(len(cblob)),
-			},
-			Layers: []ispec.Descriptor{
-				{
-					MediaType: "application/vnd.oci.image.layer.v1.tar",
-					Digest:    digest,
-					Size:      int64(len(content)),
-				},
-			},
-		}
-		manifest.SchemaVersion = 2
-
-		content, err = json.Marshal(manifest)
 		So(err, ShouldBeNil)
 
-		_, _ = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json").
+		repoName := "repo7"
-			SetBody(content).Put(url + "/v2/repo7/manifests/0.0.1")
+		err = test.UploadImage(
+			test.Image{
+				Config:   cfg,
+				Layers:   layers,
+				Manifest: manifest,
+				Tag:      "0.0.1",
+			}, url, repoName)
+		So(err, ShouldBeNil)
+
+		content, err := json.Marshal(manifest)
+		So(err, ShouldBeNil)
+		digest := godigest.FromBytes(content)
+		So(digest, ShouldNotBeNil)
 
 		err = SignImageUsingNotary("repo7:0.0.1", port)
 		So(err, ShouldBeNil)
@@ -535,7 +445,7 @@ func TestSignature(t *testing.T) {
 		str := space.ReplaceAllString(buff.String(), " ")
 		actual := strings.TrimSpace(str)
 		So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE")
-		So(actual, ShouldContainSubstring, "repo7 0.0.1 883fc0c5 true 15B")
+		So(actual, ShouldContainSubstring, "repo7 0.0.1 6742241d true 1B")
 
 		t.Log("Test getting all images using rest calls to get catalog and individual manifests")
 		cmd = MockNewImageCommand(new(searchService))
@@ -548,7 +458,7 @@ func TestSignature(t *testing.T) {
 		str = space.ReplaceAllString(buff.String(), " ")
 		actual = strings.TrimSpace(str)
 		So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE")
-		So(actual, ShouldContainSubstring, "repo7 0.0.1 883fc0c5 true 492B")
+		So(actual, ShouldContainSubstring, "repo7 0.0.1 6742241d true 447B")
 
 		err = os.Chdir(currentWorkingDir)
 		So(err, ShouldBeNil)
@@ -568,27 +478,10 @@ func TestDerivedImageList(t *testing.T) {
 	ctlr := api.NewController(conf)
 	ctlr.Config.Storage.RootDirectory = t.TempDir()
 
-	go func(controller *api.Controller) {
+	cm := test.NewControllerManager(ctlr)
-		// this blocks
-		if err := controller.Run(context.Background()); err != nil {
-			return
-		}
-	}(ctlr)
 
-	// wait till ready
+	cm.StartAndWait(conf.HTTP.Port)
-	for {
+	defer cm.StopServer()
-		_, err := resty.R().Get(url)
-		if err == nil {
-			break
-		}
-
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	defer func(controller *api.Controller) {
-		ctx := context.Background()
-		_ = controller.Server.Shutdown(ctx)
-	}(ctlr)
 
 	err := uploadManifest(url)
 	if err != nil {
@@ -666,28 +559,10 @@ func TestBaseImageList(t *testing.T) {
 	}
 	ctlr := api.NewController(conf)
 	ctlr.Config.Storage.RootDirectory = t.TempDir()
+	cm := test.NewControllerManager(ctlr)
 
-	go func(controller *api.Controller) {
+	cm.StartAndWait(conf.HTTP.Port)
-		// this blocks
+	defer cm.StopServer()
-		if err := controller.Run(context.Background()); err != nil {
-			return
-		}
-	}(ctlr)
-
-	// wait till ready
-	for {
-		_, err := resty.R().Get(url)
-		if err == nil {
-			break
-		}
-
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	defer func(controller *api.Controller) {
-		ctx := context.Background()
-		_ = controller.Server.Shutdown(ctx)
-	}(ctlr)
 
 	err := uploadManifest(url)
 	if err != nil {
@@ -1014,25 +889,9 @@ func TestServerResponseGQL(t *testing.T) {
 		}
 		ctlr := api.NewController(conf)
 		ctlr.Config.Storage.RootDirectory = t.TempDir()
-		go func(controller *api.Controller) {
+		cm := test.NewControllerManager(ctlr)
-			// this blocks
+		cm.StartAndWait(conf.HTTP.Port)
-			if err := controller.Run(context.Background()); err != nil {
+		defer cm.StopServer()
-				return
-			}
-		}(ctlr)
-		// wait till ready
-		for {
-			_, err := resty.R().Get(url)
-			if err == nil {
-				break
-			}
-
-			time.Sleep(100 * time.Millisecond)
-		}
-		defer func(controller *api.Controller) {
-			ctx := context.Background()
-			_ = controller.Server.Shutdown(ctx)
-		}(ctlr)
 
 		err := uploadManifest(url)
 		t.Logf("%s", ctlr.Config.Storage.RootDirectory)
@@ -1286,28 +1145,10 @@ func TestServerResponse(t *testing.T) {
 	}
 	ctlr := api.NewController(conf)
 	ctlr.Config.Storage.RootDirectory = t.TempDir()
+	cm := test.NewControllerManager(ctlr)
 
-	go func(controller *api.Controller) {
+	cm.StartAndWait(conf.HTTP.Port)
-		// this blocks
+	defer cm.StopServer()
-		if err := controller.Run(context.Background()); err != nil {
-			return
-		}
-	}(ctlr)
-
-	// wait till ready
-	for {
-		_, err := resty.R().Get(url)
-		if err == nil {
-			break
-		}
-
-		time.Sleep(100 * time.Millisecond)
-	}
-
-	defer func(controller *api.Controller) {
-		ctx := context.Background()
-		_ = controller.Server.Shutdown(ctx)
-	}(ctlr)
 
 	err := uploadManifest(url)
 	if err != nil {

pkg/cli/root_test.go

@@ -1,7 +1,6 @@
 package cli_test
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -10,7 +9,6 @@ import (
 	"time"
 
 	. "github.com/smartystreets/goconvey/convey"
-	"gopkg.in/resty.v1"
 
 	"zotregistry.io/zot/pkg/api"
 	"zotregistry.io/zot/pkg/api/config"
@@ -1240,21 +1238,8 @@ func TestScrub(t *testing.T) {
 			dir := t.TempDir()
 
 			controller.Config.Storage.RootDirectory = dir
-			go func(controller *api.Controller) {
+			ctrlManager := NewControllerManager(controller)
-				// this blocks
+			ctrlManager.StartAndWait(port)
-				if err := controller.Run(context.Background()); err != nil {
-					return
-				}
-			}(controller)
-			// wait till ready
-			for {
-				_, err := resty.R().Get(fmt.Sprintf("http://127.0.0.1:%s", port))
-				if err == nil {
-					break
-				}
-
-				time.Sleep(100 * time.Millisecond)
-			}
 
 			tmpfile, err := os.CreateTemp("", "zot-test*.json")
 			So(err, ShouldBeNil)
@@ -1279,10 +1264,7 @@ func TestScrub(t *testing.T) {
 			os.Args = []string{"cli_test", "scrub", tmpfile.Name()}
 			So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic)
 
-			defer func(controller *api.Controller) {
+			defer ctrlManager.StopServer()
-				ctx := context.Background()
-				_ = controller.Server.Shutdown(ctx)
-			}(controller)
 		})
 
 		Convey("no image store provided", func(c C) {

pkg/cli/stress_test.go

@@ -4,7 +4,6 @@
 package cli_test
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"os/exec"
@@ -40,7 +39,6 @@ func TestSressTooManyOpenFiles(t *testing.T) {
 		So(err, ShouldBeNil)
 
 		port := test.GetFreePort()
-		baseURL := test.GetBaseURL(port)
 		conf := config.New()
 		conf.HTTP.Port = port
 		conf.Storage.Dedupe = false
@@ -75,8 +73,9 @@ func TestSressTooManyOpenFiles(t *testing.T) {
 		t.Log("Storage root dir is: ", dir)
 		ctlr.Config.Storage.RootDirectory = dir
 
-		go startServer(ctlr)
+		ctrlManager := test.NewControllerManager(ctlr)
-		test.WaitTillServerReady(baseURL)
+		ctrlManager.StartAndWait(port)
+
 		content := fmt.Sprintf(`{
 				"storage": {
 					"rootDirectory": "%s",
@@ -132,7 +131,7 @@ func TestSressTooManyOpenFiles(t *testing.T) {
 		So(err, ShouldBeNil)
 		So(string(data), ShouldContainSubstring, "too many open files")
 
-		stopServer(ctlr)
+		ctrlManager.StopServer()
 		time.Sleep(2 * time.Second)
 
 		scrubFile, err := os.CreateTemp("", "zot-scrub*.txt")
@@ -213,18 +212,3 @@ func setMaxOpenFilesLimit(limit uint64) (uint64, error) {
 
 	return initialLimit, nil
 }
-
-func startServer(c *api.Controller) {
-	// this blocks
-	ctx := context.Background()
-	if err := c.Run(ctx); err != nil {
-		fmt.Printf("\nerror on starting zot server: %s\n", err)
-	}
-}
-
-func stopServer(c *api.Controller) {
-	ctx := context.Background()
-	if err := c.Server.Shutdown(ctx); err != nil {
-		fmt.Printf("\nerror on stopping zot server: %s\n", err)
-	}
-}