Build: clean up and document integration test convention (#58170)

* clean up and document integration test convention

* clarify integration test conventions

* clean up integration tests that don't follow convention

* mark testIntegration* functions as helpers to avoid confusion

contribute/style-guides/backend.md

@@ -40,6 +40,23 @@ The majority of our tests uses [GoConvey](http://goconvey.co/) but that's someth
 
 In the `sqlstore` package we do database operations in tests and while some might say that's not suited for unit tests. We think they are fast enough and provide a lot of value.
 
+### Integration Tests
+
+We run unit and integration tests separately, to help keep our CI pipeline running smoothly and provide a better developer experience.
+
+To properly mark a test as being an integration test, you must format your test function definition as follows, with the function name starting with `TestIntegration` and the check for `testing.Short()`:
+
+```
+func TestIntegrationFoo(t *testing.T) {
+    if testing.Short() {
+        t.Skip("skipping integration test")
+    }
+    // function body
+}
+```
+
+If you do not follow this convention, your integration test may be run twice or not run at all.
+
 ### Assertions
 
 Use respectively [`assert.*`](https://github.com/stretchr/testify#assert-package) functions to make assertions that

pkg/plugins/manager/manager_integration_test.go

@@ -49,8 +49,9 @@ import (
 )
 
 func TestIntegrationPluginManager(t *testing.T) {
-	t.Helper()
-
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	staticRootPath, err := filepath.Abs("../../../public/")
 	require.NoError(t, err)
 

pkg/services/accesscontrol/resourcepermissions/store_test.go

@@ -28,6 +28,9 @@ type setUserResourcePermissionTest struct {
 }
 
 func TestIntegrationStore_SetUserResourcePermission(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	tests := []setUserResourcePermissionTest{
 		{
 			desc:              "should set resource permission for user",
@@ -110,6 +113,9 @@ type setTeamResourcePermissionTest struct {
 }
 
 func TestIntegrationStore_SetTeamResourcePermission(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	tests := []setTeamResourcePermissionTest{
 		{
 			desc:              "should add new resource permission for team",
@@ -195,6 +201,9 @@ type setBuiltInResourcePermissionTest struct {
 }
 
 func TestIntegrationStore_SetBuiltInResourcePermission(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	tests := []setBuiltInResourcePermissionTest{
 		{
 			desc:              "should add new resource permission for builtin role",
@@ -276,6 +285,9 @@ type setResourcePermissionsTest struct {
 }
 
 func TestIntegrationStore_SetResourcePermissions(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	tests := []setResourcePermissionsTest{
 		{
 			desc:              "should set all permissions provided",
@@ -345,6 +357,9 @@ type getResourcePermissionsTest struct {
 }
 
 func TestIntegrationStore_GetResourcePermissions(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	tests := []getResourcePermissionsTest{
 		{
 			desc: "should return permissions for resource id",

pkg/services/apikey/apikeyimpl/sqlx_store_test.go

@@ -8,6 +8,9 @@ import (
 )
 
 func TestIntegrationSQLxApiKeyDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationApiKeyDataAccess(t, func(ss db.DB, cfg *setting.Cfg) store {
 		return &sqlxStore{sess: ss.GetSqlxSession(), cfg: cfg}
 	})

pkg/services/apikey/apikeyimpl/store_test.go

@@ -53,9 +53,8 @@ func seedApiKeys(t *testing.T, store store, num int) {
 }
 
 func testIntegrationApiKeyDataAccess(t *testing.T, fn getStore) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
+	t.Helper()
+
 	mockTimeNow()
 	defer resetTimeNow()
 

pkg/services/apikey/apikeyimpl/xorm_store_test.go

@@ -8,6 +8,9 @@ import (
 )
 
 func TestIntegrationXORMApiKeyDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationApiKeyDataAccess(t, func(ss db.DB, cfg *setting.Cfg) store {
 		return &sqlStore{db: ss, cfg: cfg}
 	})

pkg/services/dashboardversion/dashverimpl/sqlx_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationSQLxGetDashboardVersion(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationGetDashboardVersion(t, func(ss db.DB) store {
 		return &sqlxStore{
 			sess: ss.GetSqlxSession(),

pkg/services/dashboardversion/dashverimpl/store_test.go

@@ -19,9 +19,8 @@ import (
 type getStore func(db.DB) store
 
 func testIntegrationGetDashboardVersion(t *testing.T, fn getStore) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
+	t.Helper()
+
 	ss := db.InitTestDB(t)
 	dashVerStore := fn(ss)
 

pkg/services/dashboardversion/dashverimpl/xorm_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationXORMGetDashboardVersion(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationGetDashboardVersion(t, func(ss db.DB) store {
 		return &sqlStore{
 			db:      ss,

pkg/services/folder/folderimpl/sqlstore_test.go

@@ -18,6 +18,9 @@ import (
 )
 
 func TestIntegrationCreate(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	t.Skip("skipping until folder migration is merged")
 
 	db := sqlstore.InitTestDB(t)
@@ -153,6 +156,9 @@ func TestIntegrationCreate(t *testing.T) {
 }
 
 func TestIntegrationDelete(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	t.Skip("skipping until folder migration is merged")
 
 	db := sqlstore.InitTestDB(t)
@@ -198,6 +204,9 @@ func TestIntegrationDelete(t *testing.T) {
 }
 
 func TestIntegrationUpdate(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	t.Skip("skipping until folder migration is merged")
 
 	db := sqlstore.InitTestDB(t)
@@ -300,6 +309,9 @@ func TestIntegrationUpdate(t *testing.T) {
 }
 
 func TestIntegrationGet(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	t.Skip("skipping until folder migration is merged")
 
 	db := sqlstore.InitTestDB(t)
@@ -378,6 +390,9 @@ func TestIntegrationGet(t *testing.T) {
 }
 
 func TestIntegrationGetParents(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	t.Skip("skipping until folder migration is merged")
 
 	db := sqlstore.InitTestDB(t)
@@ -439,6 +454,9 @@ func TestIntegrationGetParents(t *testing.T) {
 }
 
 func TestIntegrationGetChildren(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	t.Skip("skipping until folder migration is merged")
 
 	db := sqlstore.InitTestDB(t)

pkg/services/ngalert/store/alert_rule_test.go

@@ -18,6 +18,9 @@ import (
 )
 
 func TestIntegrationUpdateAlertRules(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	sqlStore := db.InitTestDB(t)
 	store := DBstore{
 		SQLStore: sqlStore,
@@ -94,6 +97,9 @@ func withIntervalMatching(baseInterval time.Duration) func(*models.AlertRule) {
 }
 
 func TestIntegration_getFilterByOrgsString(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testCases := []struct {
 		testName       string
 		orgs           map[int64]struct{}

pkg/services/ngalert/store/provisioning_store_test.go

@@ -15,6 +15,9 @@ import (
 const testAlertingIntervalSeconds = 10
 
 func TestIntegrationProvisioningStore(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	store := createProvisioningStoreSut(tests.SetupTestEnv(t, testAlertingIntervalSeconds))
 
 	t.Run("Default provenance of a known type is None", func(t *testing.T) {

pkg/services/playlist/playlistimpl/sqlx_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationSQLxPlaylistDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationPlaylistDataAccess(t, func(ss db.DB) store {
 		return &sqlxStore{sess: ss.GetSqlxSession()}
 	})

pkg/services/playlist/playlistimpl/store_test.go

@@ -13,9 +13,7 @@ import (
 type getStore func(db.DB) store
 
 func testIntegrationPlaylistDataAccess(t *testing.T, fn getStore) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
+	t.Helper()
 
 	ss := db.InitTestDB(t)
 	playlistStore := fn(ss)

pkg/services/playlist/playlistimpl/xorm_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationXormPlaylistDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationPlaylistDataAccess(t, func(ss db.DB) store {
 		return &sqlStore{db: ss}
 	})

pkg/services/preference/prefimpl/sqlx_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationSQLxPreferencesDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationPreferencesDataAccess(t, func(ss db.DB) store {
 		return &sqlxStore{sess: ss.GetSqlxSession()}
 	})

pkg/services/preference/prefimpl/store_test.go

@@ -16,9 +16,8 @@ import (
 type getStore func(db.DB) store
 
 func testIntegrationPreferencesDataAccess(t *testing.T, fn getStore) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
+	t.Helper()
+
 	ss := db.InitTestDB(t)
 	prefStore := fn(ss)
 	orgNavbarPreferences := pref.NavbarPreference{

pkg/services/preference/prefimpl/xorm_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationXORMPreferencesDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationPreferencesDataAccess(t, func(ss db.DB) store {
 		return &sqlStore{db: ss}
 	})

pkg/services/publicdashboards/api/query_test.go

@@ -253,6 +253,9 @@ func getValidQueryPath(accessToken string) string {
 }
 
 func TestIntegrationUnauthenticatedUserCanGetPubdashPanelQueryData(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	db := db.InitTestDB(t)
 
 	cacheService := datasourcesService.ProvideCacheService(localcache.ProvideService(), db)

pkg/services/publicdashboards/database/database_test.go

@@ -31,6 +31,9 @@ func TestLogPrefix(t *testing.T) {
 }
 
 func TestIntegrationListPublicDashboard(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	sqlStore, cfg := db.InitTestDBwithCfg(t, db.InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagPublicDashboards}})
 	dashboardStore := dashboardsDB.ProvideDashboardStore(sqlStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore, cfg))
 	publicdashboardStore := ProvideStore(sqlStore)
@@ -64,6 +67,9 @@ func TestIntegrationListPublicDashboard(t *testing.T) {
 }
 
 func TestIntegrationFindDashboard(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -88,6 +94,9 @@ func TestIntegrationFindDashboard(t *testing.T) {
 }
 
 func TestIntegrationExistsEnabledByAccessToken(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -155,6 +164,9 @@ func TestIntegrationExistsEnabledByAccessToken(t *testing.T) {
 }
 
 func TestIntegrationExistsEnabledByDashboardUid(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -214,6 +226,9 @@ func TestIntegrationExistsEnabledByDashboardUid(t *testing.T) {
 }
 
 func TestIntegrationFindByDashboardUid(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -276,6 +291,9 @@ func TestIntegrationFindByDashboardUid(t *testing.T) {
 }
 
 func TestIntegrationFindByAccessToken(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -339,6 +357,9 @@ func TestIntegrationFindByAccessToken(t *testing.T) {
 }
 
 func TestIntegrationCreatePublicDashboard(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -406,6 +427,9 @@ func TestIntegrationCreatePublicDashboard(t *testing.T) {
 }
 
 func TestIntegrationUpdatePublicDashboard(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -497,6 +521,9 @@ func TestIntegrationUpdatePublicDashboard(t *testing.T) {
 }
 
 func TestIntegrationGetOrgIdByAccessToken(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore
@@ -563,6 +590,9 @@ func TestIntegrationGetOrgIdByAccessToken(t *testing.T) {
 }
 
 func TestIntegrationDelete(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	var sqlStore db.DB
 	var cfg *setting.Cfg
 	var dashboardStore *dashboardsDB.DashboardStore

pkg/services/querylibrary/tests/querylibrary_integration_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/tsdb/grafanads"
 )
 
-func TestCreateAndDelete(t *testing.T) {
+func TestIntegrationCreateAndDelete(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}
@@ -123,7 +123,7 @@ func createQuery(t *testing.T, ctx context.Context, testCtx testContext) string
 	return search[0].uid
 }
 
-func TestDashboardGetWithLatestSavedQueries(t *testing.T) {
+func TestIntegrationDashboardGetWithLatestSavedQueries(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}

pkg/services/sqlstore/bulk_test.go

@@ -60,7 +60,7 @@ func TestBatching(t *testing.T) {
 	})
 }
 
-func TestBulkOps(t *testing.T) {
+func TestIntegrationBulkOps(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}

pkg/services/star/starimpl/sqlx_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationSQLxUserStarsDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationUserStarsDataAccess(t, func(ss db.DB) store {
 		return &sqlxStore{sess: ss.GetSqlxSession()}
 	})

pkg/services/star/starimpl/store_test.go

@@ -13,9 +13,8 @@ import (
 type getStore func(db.DB) store
 
 func testIntegrationUserStarsDataAccess(t *testing.T, fn getStore) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
+	t.Helper()
+
 	t.Run("Testing User Stars Data Access", func(t *testing.T) {
 		ss := db.InitTestDB(t)
 		starStore := fn(ss)

pkg/services/star/starimpl/xorm_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationXormUserStarsDataAccess(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationUserStarsDataAccess(t, func(ss db.DB) store {
 		return &sqlStore{db: ss}
 	})

pkg/services/store/object/tests/server_integration_test.go

@@ -112,7 +112,7 @@ func requireVersionMatch(t *testing.T, obj *object.ObjectVersionInfo, m objectVe
 	require.True(t, len(mismatches) == 0, mismatches)
 }
 
-func TestObjectServer(t *testing.T) {
+func TestIntegrationObjectServer(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}

pkg/services/tag/tagimpl/sqlx_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationSQLxSavingTags(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationSavingTags(t, func(ss db.DB) store {
 		return &sqlxStore{sess: ss.GetSqlxSession()}
 	})

pkg/services/tag/tagimpl/store_test.go

@@ -13,9 +13,8 @@ import (
 type getStore func(db.DB) store
 
 func testIntegrationSavingTags(t *testing.T, fn getStore) {
-	if testing.Short() {
-		t.Skip("skipping integration test")
-	}
+	t.Helper()
+
 	ss := db.InitTestDB(t)
 	store := fn(ss)
 	tagPairs := []*tag.Tag{

pkg/services/tag/tagimpl/xorm_store_test.go

@@ -7,6 +7,9 @@ import (
 )
 
 func TestIntegrationXormSavingTags(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	testIntegrationSavingTags(t, func(ss db.DB) store {
 		return &sqlStore{db: ss}
 	})

pkg/tests/api/elasticsearch/elasticsearch_test.go

@@ -21,6 +21,9 @@ import (
 )
 
 func TestIntegrationElasticsearch(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		DisableAnonymous: true,
 	})

pkg/tests/api/graphite/graphite_test.go

@@ -21,6 +21,9 @@ import (
 )
 
 func TestIntegrationGraphite(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		DisableAnonymous: true,
 	})

pkg/tests/api/influxdb/influxdb_test.go

@@ -21,6 +21,9 @@ import (
 )
 
 func TestIntegrationInflux(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		DisableAnonymous: true,
 	})

pkg/tests/api/loki/loki_test.go

@@ -21,6 +21,9 @@ import (
 )
 
 func TestIntegrationLoki(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		DisableAnonymous: true,
 	})

pkg/tests/api/opentdsb/opentdsb_test.go

@@ -21,6 +21,9 @@ import (
 )
 
 func TestIntegrationOpenTSDB(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		DisableAnonymous: true,
 	})

pkg/tests/api/prometheus/prometheus_test.go

@@ -21,6 +21,9 @@ import (
 )
 
 func TestIntegrationPrometheusBuffered(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		DisableAnonymous: true,
 	})
@@ -104,6 +107,9 @@ func TestIntegrationPrometheusBuffered(t *testing.T) {
 }
 
 func TestIntegrationPrometheusClient(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping integration test")
+	}
 	dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{
 		EnableFeatureToggles: []string{"prometheusStreamingJSONParser"},
 	})

pkg/tsdb/postgres/locker_test.go

@@ -8,7 +8,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestLocker(t *testing.T) {
+func TestIntegrationLocker(t *testing.T) {
 	if testing.Short() {
 		t.Skip("Tests with Sleep")
 	}