Refactor more code to use testutils.WasCalled (#13054)

* Refactor more code to use testutils.WasCalled

* incorporate review comments

* Revert watcher_test.go changes

The file is in config package and the other tests use config_test.
So it is not visible.

config/database_test.go

@@ -513,11 +513,7 @@ func TestDatabaseStoreSet(t *testing.T) {
 		id, _ := getActualDatabaseConfig(t)
 		assert.NotEqual(t, activeId, id, "new record should have been written")
 
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config written")
-		}
 	})
 }
 
@@ -754,11 +750,7 @@ func TestDatabaseStoreLoad(t *testing.T) {
 		err = ds.Load()
 		require.NoError(t, err)
 
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config loaded")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config loaded")
-		}
 	})
 }
 

config/file_test.go

@@ -477,11 +477,7 @@ func TestFileStoreSet(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, oldCfg, retCfg)
 
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config written")
-		}
 	})
 
 	t.Run("watcher restarted", func(t *testing.T) {
@@ -513,11 +509,7 @@ func TestFileStoreSet(t *testing.T) {
 		require.NoError(t, err)
 
 		ioutil.WriteFile(path, cfgData, 0644)
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config written")
-		}
 	})
 }
 
@@ -749,11 +741,7 @@ func TestFileStoreLoad(t *testing.T) {
 		err = fs.Load()
 		require.NoError(t, err)
 
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config loaded")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config loaded")
-		}
 	})
 }
 
@@ -786,11 +774,7 @@ func TestFileStoreWatcherEmitter(t *testing.T) {
 		require.NoError(t, err)
 
 		ioutil.WriteFile(path, cfgData, 0644)
-		select {
+		require.False(t, wasCalled(called, 1*time.Second), "callback should not have been called since watching disabled")
-		case <-called:
-			require.Fail(t, "callback should not have been called since watching disabled")
-		case <-time.After(1 * time.Second):
-		}
 	})
 
 	t.Run("enabled", func(t *testing.T) {
@@ -809,11 +793,7 @@ func TestFileStoreWatcherEmitter(t *testing.T) {
 		require.NoError(t, err)
 
 		ioutil.WriteFile(path, cfgData, 0644)
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config written")
-		}
 	})
 }
 
@@ -1126,3 +1106,14 @@ func TestFileStoreString(t *testing.T) {
 
 	assert.Equal(t, "file://"+path, fs.String())
 }
+
+// wasCalled reports whether a given callback channel was called
+// within the specified time duration or not.
+func wasCalled(c chan bool, duration time.Duration) bool {
+	select {
+	case <-c:
+		return true
+	case <-time.After(duration):
+	}
+	return false
+}

config/memory_test.go

@@ -215,11 +215,7 @@ func TestMemoryStoreSet(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, oldCfg, retCfg)
 
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config written")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config written")
-		}
 	})
 }
 
@@ -268,11 +264,7 @@ func TestMemoryStoreLoad(t *testing.T) {
 		err = ms.Load()
 		require.NoError(t, err)
 
-		select {
+		require.True(t, wasCalled(called, 5*time.Second), "callback should have been called when config loaded")
-		case <-called:
-		case <-time.After(5 * time.Second):
-			require.Fail(t, "callback should have been called when config loaded")
-		}
 	})
 }
 

config/watcher_test.go

@@ -11,8 +11,6 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
-
-	"github.com/mattermost/mattermost-server/utils/testutils"
 )
 
 func TestWatcherInvalidDirectory(t *testing.T) {
@@ -49,9 +47,17 @@ func TestWatcher(t *testing.T) {
 
 	// Write to a different file
 	ioutil.WriteFile(filepath.Join(tempDir, "unrelated"), []byte("data"), 0644)
-	require.False(t, testutils.WasCalled(called, 1*time.Second), "callback should not have been called for unrelated file")
+	select {
+	case <-called:
+		t.Fatal("callback should not have been called for unrelated file")
+	case <-time.After(1 * time.Second):
+	}
 
 	// Write to the watched file
 	ioutil.WriteFile(f.Name(), []byte("data"), 0644)
-	require.True(t, testutils.WasCalled(called, 5*time.Second), "callback should have been called when file written")
+	select {
+	case <-called:
+	case <-time.After(5 * time.Second):
+		t.Fatal("callback should have been called when file written")
+	}
 }

utils/testutils/testutils.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"time"
 
 	"github.com/mattermost/mattermost-server/utils/fileutils"
 )
@@ -28,15 +27,3 @@ func ReadTestFile(name string) ([]byte, error) {
 		return data.Bytes(), nil
 	}
 }
-
-// WasCalled reports whether a given callback channel was called
-// within the specified time duration or not.
-func WasCalled(c chan bool, duration time.Duration) bool {
-	wasCalled := false
-	select {
-	case <-c:
-		wasCalled = true
-	case <-time.After(duration):
-	}
-	return wasCalled
-}