Tests: Batch of GoConvey to Testify conversions (#27008)

pkg/api/http_server_test.go

@@ -3,28 +3,27 @@ package api
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+
 	"github.com/grafana/grafana/pkg/setting"
-	. "github.com/smartystreets/goconvey/convey"
 )
 
-func TestHTTPServer(t *testing.T) {
-	Convey("Given a HTTPServer", t, func() {
+func TestHTTPServer_MetricsBasicAuth(t *testing.T) {
 	ts := &HTTPServer{
 		Cfg: setting.NewCfg(),
 	}
 
-		Convey("Given that basic auth on the metrics endpoint is enabled", func() {
+	t.Run("enabled", func(t *testing.T) {
 		ts.Cfg.MetricsEndpointBasicAuthUsername = "foo"
 		ts.Cfg.MetricsEndpointBasicAuthPassword = "bar"
 
-			So(ts.metricsEndpointBasicAuthEnabled(), ShouldBeTrue)
+		assert.True(t, ts.metricsEndpointBasicAuthEnabled())
 	})
 
-		Convey("Given that basic auth on the metrics endpoint is disabled", func() {
+	t.Run("disabled", func(t *testing.T) {
 		ts.Cfg.MetricsEndpointBasicAuthUsername = ""
 		ts.Cfg.MetricsEndpointBasicAuthPassword = ""
 
-			So(ts.metricsEndpointBasicAuthEnabled(), ShouldBeFalse)
-		})
+		assert.False(t, ts.metricsEndpointBasicAuthEnabled())
 	})
 }

pkg/cmd/grafana-cli/commands/ls_command.go

@@ -12,9 +12,13 @@ import (
 
 var ls_getPlugins func(path string) []models.InstalledPlugin = services.GetLocalPlugins
 
+var (
+	errMissingPathFlag = errors.New("missing path flag")
+	errNotDirectory    = errors.New("plugin path is not a directory")
+)
 var validateLsCommand = func(pluginDir string) error {
 	if pluginDir == "" {
-		return errors.New("missing path flag")
+		return errMissingPathFlag
 	}
 
 	logger.Debug("plugindir: " + pluginDir + "\n")
@@ -24,7 +28,7 @@ var validateLsCommand = func(pluginDir string) error {
 	}
 
 	if !pluginDirInfo.IsDir() {
-		return errors.New("plugin path is not a directory")
+		return errNotDirectory
 	}
 
 	return nil

pkg/cmd/grafana-cli/commands/ls_command_test.go

@@ -4,67 +4,81 @@ import (
 	"errors"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/cmd/grafana-cli/commands/commandstest"
 	"github.com/grafana/grafana/pkg/cmd/grafana-cli/services"
-	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestMissingPath(t *testing.T) {
+	tests := []struct {
+		description string
+		cliContext  map[string]string
+		ioHelper    *commandstest.FakeIoUtil
+		error       error
+	}{
+		{
+			description: "missing path flag",
+			cliContext:  make(map[string]string),
+			ioHelper:    &commandstest.FakeIoUtil{},
+			error:       errMissingPathFlag,
+		},
+		{
+			description: "not a directory",
+			cliContext:  map[string]string{"pluginsDir": "/var/lib/grafana/plugins/notadir.txt"},
+			ioHelper:    &commandstest.FakeIoUtil{FakeIsDirectory: false},
+			error:       errNotDirectory,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.description, func(t *testing.T) {
+			origIoHelper := services.IoHelper
+			services.IoHelper = tc.ioHelper
+			t.Cleanup(func() {
+				services.IoHelper = origIoHelper
+			})
+
+			c, err := commandstest.NewCliContext(tc.cliContext)
+			require.NoError(t, err)
+
+			cmd := Command{}
+			err = cmd.lsCommand(c)
+			assert.Equal(t, tc.error, err)
+		})
+	}
+}
+
+func TestValidateLsCommand_override(t *testing.T) {
+	expected := errors.New("dummy error")
+	t.Run("override validateLsCommand", func(t *testing.T) {
 		var org = validateLsCommand
 
-	Convey("ls command", t, func() {
+		t.Cleanup(func() {
 			validateLsCommand = org
-
-		Convey("Missing path flag", func() {
-			cmd := Command{}
-			c, err := commandstest.NewCliContext(map[string]string{})
-			So(err, ShouldBeNil)
-			services.IoHelper = &commandstest.FakeIoUtil{}
-
-			Convey("should return error", func() {
-				err := cmd.lsCommand(c)
-				So(err, ShouldBeError, "missing path flag")
-			})
 		})
 
-		Convey("Path is not a directory", func() {
 		c, err := commandstest.NewCliContext(map[string]string{"path": "/var/lib/grafana/plugins"})
-			So(err, ShouldBeNil)
-			services.IoHelper = &commandstest.FakeIoUtil{
-				FakeIsDirectory: false,
-			}
-			cmd := Command{}
-
-			Convey("should return error", func() {
-				err := cmd.lsCommand(c)
-				So(err, ShouldNotBeNil)
-			})
-		})
-
-		Convey("can override validateLsCommand", func() {
-			c, err := commandstest.NewCliContext(map[string]string{"path": "/var/lib/grafana/plugins"})
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
 
 		validateLsCommand = func(pluginDir string) error {
-				return errors.New("dummy error")
+			return expected
 		}
 
-			Convey("should return error", func() {
 		cmd := Command{}
-				err := cmd.lsCommand(c)
-				So(err.Error(), ShouldEqual, "dummy error")
-			})
+		err = cmd.lsCommand(c)
+		assert.Error(t, err)
+		assert.Equal(t, expected, err, "can override validateLsCommand")
 	})
 
-		Convey("Validate that validateLsCommand is reset", func() {
+	// meta-test for test cleanup of global variable
+	t.Run("validateLsCommand reset after test", func(t *testing.T) {
 		c, err := commandstest.NewCliContext(map[string]string{"path": "/var/lib/grafana/plugins"})
-			So(err, ShouldBeNil)
-			cmd := Command{}
+		require.NoError(t, err)
 
-			Convey("should return error", func() {
-				err := cmd.lsCommand(c)
-				So(err.Error(), ShouldNotEqual, "dummy error")
-			})
-		})
+		cmd := Command{}
+		err = cmd.lsCommand(c)
+		assert.NotEqual(t, err, expected, "validateLsCommand is reset")
 	})
 }

pkg/components/apikeygen/apikeygen_test.go

@@ -3,25 +3,23 @@ package apikeygen
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/util"
-	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestApiKeyGen(t *testing.T) {
-	Convey("When generating new api key", t, func() {
 	result, err := New(12, "Cool key")
-		So(err, ShouldBeNil)
+	require.NoError(t, err)
 
-		So(result.ClientSecret, ShouldNotBeEmpty)
-		So(result.HashedKey, ShouldNotBeEmpty)
+	assert.NotEmpty(t, result.ClientSecret)
+	assert.NotEmpty(t, result.HashedKey)
 
-		Convey("can decode key", func() {
 	keyInfo, err := Decode(result.ClientSecret)
-			So(err, ShouldBeNil)
+	require.NoError(t, err)
 
 	keyHashed, err := util.EncodePassword(keyInfo.Key, keyInfo.Name)
-			So(err, ShouldBeNil)
-			So(keyHashed, ShouldEqual, result.HashedKey)
-		})
-	})
+	require.NoError(t, err)
+	assert.Equal(t, result.HashedKey, keyHashed)
 }

pkg/components/dashdiffs/formatter_test.go

@@ -3,8 +3,10 @@ package dashdiffs
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/components/simplejson"
-	. "github.com/smartystreets/goconvey/convey"
 )
 
 func TestDiff(t *testing.T) {
@@ -60,21 +62,20 @@ func TestDiff(t *testing.T) {
 		}`
 	)
 
-	Convey("Testing dashboard diffs", t, func() {
 	// Compute the diff between the two JSON objects
 	baseData, err := simplejson.NewJson([]byte(leftJSON))
-		So(err, ShouldBeNil)
+	require.NoError(t, err)
 
 	newData, err := simplejson.NewJson([]byte(rightJSON))
-		So(err, ShouldBeNil)
+	require.NoError(t, err)
 
 	left, jsonDiff, err := getDiff(baseData, newData)
-		So(err, ShouldBeNil)
+	require.NoError(t, err)
 
-		Convey("The JSONFormatter should produce the expected JSON tokens", func() {
+	t.Run("JSONFormatter produces expected JSON tokens", func(t *testing.T) {
 		f := NewJSONFormatter(left)
 		_, err := f.Format(jsonDiff)
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
 
 		// Total up the change types. If the number of different change
 		// types is correct, it means that the diff is producing correct
@@ -94,13 +95,13 @@ func TestDiff(t *testing.T) {
 			ChangeNew:       5,
 			ChangeUnchanged: 5,
 		}
-			So(changeCounts, ShouldResemble, expectedChangeCounts)
+		assert.EqualValues(t, expectedChangeCounts, changeCounts)
 	})
 
-		Convey("The BasicFormatter should produce the expected BasicBlocks", func() {
+	t.Run("BasicFormatter produces expected BasicBlocks", func(t *testing.T) {
 		f := NewBasicFormatter(left)
 		_, err := f.Format(jsonDiff)
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
 
 		bd := &BasicDiff{}
 		blocks := bd.Basic(f.jsonDiff.Lines)
@@ -124,7 +125,6 @@ func TestDiff(t *testing.T) {
 			ChangeDeleted: 1,
 			ChangeOld:     3,
 		}
-			So(changeCounts, ShouldResemble, expectedChangeCounts)
-		})
+		assert.EqualValues(t, expectedChangeCounts, changeCounts)
 	})
 }

pkg/infra/log/file_test.go

@@ -4,7 +4,9 @@ import (
 	"os"
 	"testing"
 
-	. "github.com/smartystreets/goconvey/convey"
+	"github.com/stretchr/testify/require"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func (w *FileLogWriter) WriteLine(line string) error {
@@ -17,30 +19,29 @@ func (w *FileLogWriter) WriteLine(line string) error {
 }
 
 func TestLogFile(t *testing.T) {
-	Convey("When logging to file", t, func() {
 	fileLogWrite := NewFileWriter()
-		So(fileLogWrite, ShouldNotBeNil)
+	require.NotNil(t, fileLogWrite)
+
+	t.Cleanup(func() {
+		fileLogWrite.Close()
+		err := os.Remove(fileLogWrite.Filename)
+		require.NoError(t, err)
+	})
 
 	fileLogWrite.Filename = "grafana_test.log"
 	err := fileLogWrite.Init()
-		So(err, ShouldBeNil)
+	require.NoError(t, err)
 
-		Convey("Log file is empty", func() {
-			So(fileLogWrite.maxlines_curlines, ShouldEqual, 0)
-		})
+	assert.Zero(t, fileLogWrite.maxlines_curlines)
 
-		Convey("Logging should add lines", func() {
+	t.Run("adding lines", func(t *testing.T) {
 		err := fileLogWrite.WriteLine("test1\n")
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
 		err = fileLogWrite.WriteLine("test2\n")
-			So(err, ShouldBeNil)
+		require.NoError(t, err)
 		err = fileLogWrite.WriteLine("test3\n")
-			So(err, ShouldBeNil)
-			So(fileLogWrite.maxlines_curlines, ShouldEqual, 3)
-		})
+		require.NoError(t, err)
 
-		fileLogWrite.Close()
-		err = os.Remove(fileLogWrite.Filename)
-		So(err, ShouldBeNil)
+		assert.Equal(t, 3, fileLogWrite.maxlines_curlines)
 	})
 }

pkg/infra/log/log_writer_test.go

@@ -3,16 +3,14 @@ package log
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/inconshreveable/log15"
-	. "github.com/smartystreets/goconvey/convey"
 )
 
 type FakeLogger struct {
-	debug string
-	info  string
-	warn  string
-	err   string
-	crit  string
+	m map[string]string
 }
 
 func (f *FakeLogger) New(ctx ...interface{}) log15.Logger {
@@ -20,23 +18,23 @@ func (f *FakeLogger) New(ctx ...interface{}) log15.Logger {
 }
 
 func (f *FakeLogger) Debug(msg string, ctx ...interface{}) {
-	f.debug = msg
+	f.m["debug"] = msg
 }
 
 func (f *FakeLogger) Info(msg string, ctx ...interface{}) {
-	f.info = msg
+	f.m["info"] = msg
 }
 
 func (f *FakeLogger) Warn(msg string, ctx ...interface{}) {
-	f.warn = msg
+	f.m["warn"] = msg
 }
 
 func (f *FakeLogger) Error(msg string, ctx ...interface{}) {
-	f.err = msg
+	f.m["err"] = msg
 }
 
 func (f *FakeLogger) Crit(msg string, ctx ...interface{}) {
-	f.crit = msg
+	f.m["crit"] = msg
 }
 
 func (f *FakeLogger) GetHandler() log15.Handler {
@@ -45,72 +43,80 @@ func (f *FakeLogger) GetHandler() log15.Handler {
 
 func (f *FakeLogger) SetHandler(l log15.Handler) {}
 
-func TestLogWriter(t *testing.T) {
-	Convey("When writing to a LogWriter", t, func() {
-		Convey("Should write using the correct level [crit]", func() {
-			fake := &FakeLogger{}
+func TestLogWriter_level(t *testing.T) {
+	tests := []struct {
+		description      string
+		logger           string
+		prefix           string
+		level            Lvl
+		input            []byte
+		expectedConsumed int
+		expectedOutput   string
+	}{
+		{
+			description:      "level crit",
+			logger:           "crit",
+			input:            []byte("crit"),
+			level:            LvlCrit,
+			expectedConsumed: 4,
+			expectedOutput:   "crit",
+		},
+		{
+			description:      "level error",
+			logger:           "err",
+			input:            []byte("error"),
+			level:            LvlError,
+			expectedConsumed: 5,
+			expectedOutput:   "error",
+		},
+		{
+			description:      "level warn",
+			logger:           "warn",
+			input:            []byte("warn"),
+			level:            LvlWarn,
+			expectedConsumed: 4,
+			expectedOutput:   "warn",
+		},
+		{
+			description:      "level info",
+			logger:           "info",
+			input:            []byte("info"),
+			level:            LvlInfo,
+			expectedConsumed: 4,
+			expectedOutput:   "info",
+		},
+		{
+			description:      "level debug",
+			logger:           "debug",
+			input:            []byte("debug"),
+			level:            LvlDebug,
+			expectedConsumed: 5,
+			expectedOutput:   "debug",
+		},
+		{
+			description:      "prefix",
+			logger:           "debug",
+			input:            []byte("debug"),
+			prefix:           "prefix",
+			level:            LvlDebug,
+			expectedConsumed: 5,
+			expectedOutput:   "prefixdebug",
+		},
+	}
 
-			crit := NewLogWriter(fake, LvlCrit, "")
-			n, err := crit.Write([]byte("crit"))
+	for _, tc := range tests {
+		tc := tc // to avoid timing issues
 
-			So(n, ShouldEqual, 4)
-			So(err, ShouldBeNil)
-			So(fake.crit, ShouldEqual, "crit")
-		})
-
-		Convey("Should write using the correct level [error]", func() {
-			fake := &FakeLogger{}
-
-			crit := NewLogWriter(fake, LvlError, "")
-			n, err := crit.Write([]byte("error"))
-
-			So(n, ShouldEqual, 5)
-			So(err, ShouldBeNil)
-			So(fake.err, ShouldEqual, "error")
-		})
-
-		Convey("Should write using the correct level [warn]", func() {
-			fake := &FakeLogger{}
-
-			crit := NewLogWriter(fake, LvlWarn, "")
-			n, err := crit.Write([]byte("warn"))
-
-			So(n, ShouldEqual, 4)
-			So(err, ShouldBeNil)
-			So(fake.warn, ShouldEqual, "warn")
-		})
-
-		Convey("Should write using the correct level [info]", func() {
-			fake := &FakeLogger{}
-
-			crit := NewLogWriter(fake, LvlInfo, "")
-			n, err := crit.Write([]byte("info"))
-
-			So(n, ShouldEqual, 4)
-			So(err, ShouldBeNil)
-			So(fake.info, ShouldEqual, "info")
-		})
-
-		Convey("Should write using the correct level [debug]", func() {
-			fake := &FakeLogger{}
-
-			crit := NewLogWriter(fake, LvlDebug, "")
-			n, err := crit.Write([]byte("debug"))
-
-			So(n, ShouldEqual, 5)
-			So(err, ShouldBeNil)
-			So(fake.debug, ShouldEqual, "debug")
-		})
-
-		Convey("Should prefix the output with the prefix", func() {
-			fake := &FakeLogger{}
-
-			crit := NewLogWriter(fake, LvlDebug, "prefix")
-			n, err := crit.Write([]byte("debug"))
-
-			So(n, ShouldEqual, 5) // n is how much of input consumed
-			So(err, ShouldBeNil)
-			So(fake.debug, ShouldEqual, "prefixdebug")
-		})
+		t.Run(tc.description, func(t *testing.T) {
+			t.Parallel()
+			fake := &FakeLogger{m: map[string]string{}}
+
+			w := NewLogWriter(fake, tc.level, tc.prefix)
+			n, err := w.Write(tc.input)
+			require.NoError(t, err)
+
+			assert.Equal(t, tc.expectedConsumed, n)
+			assert.Equal(t, tc.expectedOutput, fake.m[tc.logger])
 		})
+	}
 }

pkg/infra/serverlock/serverlock_test.go

@@ -4,9 +4,11 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	. "github.com/smartystreets/goconvey/convey"
 )
 
 func createTestableServerLock(t *testing.T) *ServerLockService {
@@ -21,35 +23,36 @@ func createTestableServerLock(t *testing.T) *ServerLockService {
 }
 
 func TestServerLock(t *testing.T) {
-	Convey("Server lock", t, func() {
 	sl := createTestableServerLock(t)
 	operationUID := "test-operation"
 
 	first, err := sl.getOrCreate(context.Background(), operationUID)
-		So(err, ShouldBeNil)
+	require.NoError(t, err)
+
+	t.Run("trying to create three new row locks", func(t *testing.T) {
+		expectedLastExecution := first.LastExecution
+		var latest *serverLock
 
-		lastExecution := first.LastExecution
-		Convey("trying to create three new row locks", func() {
 		for i := 0; i < 3; i++ {
-				first, err = sl.getOrCreate(context.Background(), operationUID)
-				So(err, ShouldBeNil)
-				So(first.OperationUid, ShouldEqual, operationUID)
-				So(first.Id, ShouldEqual, 1)
+			latest, err = sl.getOrCreate(context.Background(), operationUID)
+			require.NoError(t, err)
+			assert.Equal(t, operationUID, first.OperationUid)
+			assert.Equal(t, int64(1), first.Id)
 		}
 
-			Convey("Should not create new since lock already exist", func() {
-				So(lastExecution, ShouldEqual, first.LastExecution)
-			})
+		assert.Equal(t,
+			expectedLastExecution,
+			latest.LastExecution,
+			"latest execution should not have changed")
 	})
 
-		Convey("Should be able to create lock on first row", func() {
+	t.Run("create lock on first row", func(t *testing.T) {
 		gotLock, err := sl.acquireLock(context.Background(), first)
-			So(err, ShouldBeNil)
-			So(gotLock, ShouldBeTrue)
+		require.NoError(t, err)
+		assert.True(t, gotLock)
 
 		gotLock, err = sl.acquireLock(context.Background(), first)
-			So(err, ShouldBeNil)
-			So(gotLock, ShouldBeFalse)
-		})
+		require.NoError(t, err)
+		assert.False(t, gotLock)
 	})
 }