lib/bakcup/azremote: follow-up after 5fd3aef549

Simplify tests by converting them to f-tests.

5fd3aef549
Signed-off-by: hagen1778 <roman@victoriametrics.com>

lib/backup/azremote/azblob.go

@@ -33,26 +33,6 @@ const (
 	storageErrorCodeBlobNotFound = "BlobNotFound"
 )
 
-var (
-	errNoCredentials = fmt.Errorf(
-		`failed to detect credentials for AZBlob. 
-Ensure that one of the options is set: connection string at %q; shared key at %q and %q; account name at %q and set %q to "true"`,
-		envStorageAccCs,
-		envStorageAcctName,
-		envStorageAccKey,
-		envStorageAcctName,
-		envStorageDefault,
-	)
-
-	errInvalidCredentials = fmt.Errorf("failed to process credentials: only one of %s, %s and %s, or %s and %s can be specified",
-		envStorageAccCs,
-		envStorageAcctName,
-		envStorageAccKey,
-		envStorageAcctName,
-		envStorageDefault,
-	)
-)
-
 // FS represents filesystem for backups in Azure Blob Storage.
 //
 // Init must be called before calling other FS methods.
@@ -109,7 +89,13 @@ func (fs *FS) newClient() (*service.Client, error) {
 	switch {
 	// can't specify any combination of more than one credential
 	case moreThanOne(hasConnString, (hasAccountName && hasAccountKey), (useDefault == "true" && hasAccountName)):
-		return nil, errInvalidCredentials
+		return nil, fmt.Errorf("failed to process credentials: only one of %s, %s and %s, or %s and %s can be specified",
+			envStorageAccCs,
+			envStorageAcctName,
+			envStorageAccKey,
+			envStorageAcctName,
+			envStorageDefault,
+		)
 	case hasConnString:
 		logger.Infof("Creating AZBlob service client from connection string")
 		return service.NewClientFromConnectionString(connString, nil)
@@ -128,7 +114,15 @@ func (fs *FS) newClient() (*service.Client, error) {
 		}
 		return service.NewClient(serviceURL, creds, nil)
 	default:
-		return nil, errNoCredentials
+		return nil, fmt.Errorf(
+			`failed to detect credentials for AZBlob. 
+Ensure that one of the options is set: connection string at %q; shared key at %q and %q; account name at %q and set %q to "true"`,
+			envStorageAccCs,
+			envStorageAcctName,
+			envStorageAccKey,
+			envStorageAcctName,
+			envStorageDefault,
+		)
 	}
 }
 

lib/backup/azremote/azblob_test.go

@@ -1,155 +1,59 @@
 package azremote
 
 import (
-	"bytes"
-	"errors"
 	"strings"
 	"testing"
-
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 )
 
 func Test_cleanDirectory(t *testing.T) {
-	cases := map[string]struct {
+	f := func(dir, exp string) {
-		Dir         string
+		t.Helper()
-		ExpectedDir string
+		got := cleanDirectory(dir)
-	}{
+		if got != exp {
-		"dir / prefix is removed": {
+			t.Errorf("expected dir %q, got %q", exp, got)
-			Dir:         "/foo/",
+		}
-			ExpectedDir: "foo/",
-		},
-		"multiple dir prefix / is removed": {
-			Dir:         "//foo/",
-			ExpectedDir: "foo/",
-		},
-		"suffix is added": {
-			Dir:         "foo",
-			ExpectedDir: "foo/",
-		},
-	}
-
-	for name, test := range cases {
-		t.Run(name, func(t *testing.T) {
-			dir := cleanDirectory(test.Dir)
-
-			if dir != test.ExpectedDir {
-				t.Errorf("expected dir %q, got %q", test.ExpectedDir, dir)
-			}
-		})
 	}
+	f("/foo/", "foo/")
+	f("//foo/", "foo/")
+	f("foo", "foo/")
 }
 
 func Test_FSInit(t *testing.T) {
-	cases := map[string]struct {
+	f := func(expErr string, params ...string) {
-		IgnoreFakeEnv bool
+		t.Helper()
-		Env           testEnv
-		ExpectedErr   error
-		ExpectedLogs  []string
-	}{
-		"connection string env var is used": {
-			Env: map[string]string{
-				envStorageAccCs: "BlobEndpoint=https://test.blob.core.windows.net/;SharedAccessSignature=",
-			},
-			ExpectedLogs: []string{`Creating AZBlob service client from connection string`},
-		},
-		"base envtemplate package is used and connection string err bubbles": {
-			IgnoreFakeEnv: true,
-			Env: map[string]string{
-				envStorageAccCs: "BlobEndpoint=https://test.blob.core.windows.net/;SharedAccessSignature=",
-			},
-			ExpectedErr: errNoCredentials,
-		},
-		"only storage account name is an err": {
-			Env: map[string]string{
-				envStorageAcctName: "test",
-			},
-			ExpectedErr: errNoCredentials,
-		},
-		"uses shared key credential": {
-			Env: map[string]string{
-				envStorageAcctName: "test",
-				envStorageAccKey:   "dGVhcG90Cg==",
-			},
-			ExpectedLogs: []string{`Creating AZBlob service client from account name and key`},
-		},
-		"allows overriding domain name with account name and key": {
-			Env: map[string]string{
-				envStorageAcctName: "test",
-				envStorageAccKey:   "dGVhcG90Cg==",
-				envStorageDomain:   "foo.bar",
-			},
-			ExpectedLogs: []string{
-				`Creating AZBlob service client from account name and key`,
-				`Overriding default Azure blob domain with "foo.bar"`,
-			},
-		},
-		"can't specify both connection string and shared key": {
-			Env: map[string]string{
-				envStorageAccCs:    "teapot",
-				envStorageAcctName: "test",
-				envStorageAccKey:   "dGVhcG90Cg==",
-			},
-			ExpectedErr: errInvalidCredentials,
-		},
-		"just use default is an err": {
-			Env: map[string]string{
-				envStorageDefault: "true",
-			},
-			ExpectedErr: errNoCredentials,
-		},
-		"uses default credential": {
-			Env: map[string]string{
-				envStorageDefault:  "true",
-				envStorageAcctName: "test",
-			},
-			ExpectedLogs: []string{`Creating AZBlob service client from default credential`},
-		},
-	}
 
-	for name, test := range cases {
+		env := make(testEnv)
-		t.Run(name, func(t *testing.T) {
+		for i := 0; i < len(params); i += 2 {
-			tlog := &testLogger{}
+			env[params[i]] = params[i+1]
+		}
 
-			logger.SetOutputForTests(tlog)
+		fs := &FS{Dir: "foo"}
-			t.Cleanup(logger.ResetOutputForTest)
+		fs.env = env.LookupEnv
-
+		err := fs.Init()
-			fs := &FS{Dir: "foo"}
+		if err != nil {
-			if test.Env != nil && !test.IgnoreFakeEnv {
+			if expErr == "" {
-				fs.env = test.Env.LookupEnv
+				t.Fatalf("unexpected error %v", err)
 			}
-
+			if !strings.Contains(err.Error(), expErr) {
-			err := fs.Init()
+				t.Fatalf("expected error: \n%q, \ngot: \n%v", expErr, err)
-			if err != nil && !errors.Is(err, test.ExpectedErr) {
-				t.Errorf("expected error %q, got %q", test.ExpectedErr, err)
 			}
-
+			return
-			tlog.MustContain(t, test.ExpectedLogs...)
+		}
-		})
+		if expErr != "" {
-	}
+			t.Fatalf("expected to have an error %q, instead got nil", expErr)
-}
-
-type testLogger struct {
-	buf *bytes.Buffer
-}
-
-func (l *testLogger) Write(p []byte) (n int, err error) {
-	if l.buf == nil {
-		l.buf = &bytes.Buffer{}
-	}
-
-	return l.buf.Write(p)
-}
-
-func (l *testLogger) MustContain(t *testing.T, vals ...string) {
-	t.Helper()
-
-	contents := l.buf.String()
-
-	for _, val := range vals {
-		if !strings.Contains(contents, val) {
-			t.Errorf("expected log to contain %q, got %q", val, l.buf.String())
 		}
 	}
+
+	f("", envStorageAccCs, "BlobEndpoint=https://test.blob.core.windows.net/;SharedAccessSignature=")
+	f("", envStorageAcctName, "test", envStorageAccKey, "dGVhcG90Cg==")
+	f("", envStorageDefault, "true", envStorageAcctName, "test")
+	f("", envStorageAcctName, "test", envStorageAccKey, "dGVhcG90Cg==", envStorageDomain, "foo.bar")
+
+	f("failed to detect credentials for AZBlob")
+	f("failed to detect credentials for AZBlob", envStorageAcctName, "test")
+	f("failed to create Shared Key", envStorageAcctName, "", envStorageAccKey, "!")
+	f("connection string is either blank or malformed", envStorageAccCs, "")
+	f("failed to process credentials: only one of", envStorageAccCs, "teapot", envStorageAcctName, "test", envStorageAccKey, "dGVhcG90Cg==")
 }
 
 type testEnv map[string]string