From 601f7cda348fcebc3bac9f9fb0efc0f6cdaa67d5 Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Mon, 6 Oct 2025 10:31:30 +0200 Subject: [PATCH] CloudMigrations: Increase timeout of eventual checks and add debug message in flaky test (#112042) * CloudMigrations: Remove unused param in test setup * CloudMigrations: Increase timeout of eventual checks and add debug message --- .../cloudmigrationimpl/cloudmigration_test.go | 56 ++++++++++--------- .../snapshot_mgmt_alerts_test.go | 18 +++--- 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go index e430611f3cd..f30feca4ee2 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/cloudmigration_test.go @@ -14,12 +14,10 @@ import ( "github.com/google/uuid" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/tracing" @@ -63,7 +61,7 @@ func Test_NoopServiceDoesNothing(t *testing.T) { func Test_CreateGetAndDeleteToken(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false) + s := setUpServiceTest(t) createResp, err := s.CreateToken(context.Background()) assert.NoError(t, err) @@ -88,7 +86,7 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { t.Parallel() setupTest := func(ctx context.Context) (service *Service, snapshotUID string, sessionUID string) { - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) gmsClientFake := &gmsClientMock{} s.gmsClient = gmsClientFake @@ -365,7 +363,7 @@ func Test_GetSnapshotStatusFromGMS(t *testing.T) { func Test_OnlyQueriesStatusFromGMSWhenRequired(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) gmsClientMock := &gmsClientMock{ getSnapshotResponse: &cloudmigration.GetSnapshotStatusResponse{ @@ -427,14 +425,29 @@ func Test_OnlyQueriesStatusFromGMSWhenRequired(t *testing.T) { Status: status, }) assert.NoError(t, err) - _, err := s.GetSnapshot(context.Background(), cloudmigration.GetSnapshotsQuery{ + snapshot, err := s.GetSnapshot(context.Background(), cloudmigration.GetSnapshotsQuery{ SnapshotUID: uid, SessionUID: sess.UID, }) assert.NoError(t, err) - require.Eventually(t, func() bool { return gmsClientMock.GetSnapshotStatusCallCount() == i+1 }, time.Second, 10*time.Millisecond) + assert.Equal(t, status, snapshot.Status) + + require.Eventually( + t, + func() bool { return gmsClientMock.GetSnapshotStatusCallCount() == i+1 }, + 2*time.Second, + 100*time.Millisecond, + "GMS client mock GetSnapshotStatus count: %d", gmsClientMock.GetSnapshotStatusCallCount(), + ) } - assert.Never(t, func() bool { return gmsClientMock.GetSnapshotStatusCallCount() > 2 }, time.Second, 10*time.Millisecond) + + assert.Never( + t, + func() bool { return gmsClientMock.GetSnapshotStatusCallCount() > 2 }, + 2*time.Second, + 100*time.Millisecond, + "GMS client mock GetSnapshotStatus called more than expected: %d times", gmsClientMock.GetSnapshotStatusCallCount(), + ) } // Implementation inspired by ChatGPT, OpenAI's language model. @@ -463,7 +476,7 @@ func Test_SortFolders(t *testing.T) { func TestDeleteSession(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{UserUID: "user123"} t.Run("when deleting a session that does not exist in the database, it returns an error", func(t *testing.T) { @@ -515,7 +528,7 @@ func TestReportEvent(t *testing.T) { gmsMock := &gmsClientMock{} - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) s.gmsClient = gmsMock require.NotPanics(t, func() { @@ -533,7 +546,7 @@ func TestReportEvent(t *testing.T) { gmsMock := &gmsClientMock{} - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) s.gmsClient = gmsMock require.NotPanics(t, func() { @@ -547,7 +560,7 @@ func TestReportEvent(t *testing.T) { func TestGetFolderNamesForFolderUIDs(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -616,7 +629,7 @@ func TestGetParentNames(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -705,7 +718,7 @@ func TestGetParentNames(t *testing.T) { func TestGetLibraryElementsCommands(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -771,7 +784,7 @@ func TestIsPublicSignatureType(t *testing.T) { func TestGetPlugins(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) @@ -869,7 +882,7 @@ func TestGetPlugins(t *testing.T) { type configOverrides func(c *setting.Cfg) -func setUpServiceTest(t *testing.T, withDashboardMock bool, cfgOverrides ...configOverrides) cloudmigration.Service { +func setUpServiceTest(t *testing.T, cfgOverrides ...configOverrides) cloudmigration.Service { secretsService := secretsfakes.NewFakeSecretsService() rr := routing.NewRouteRegister() tracer := tracing.InitializeTracerForTest() @@ -888,17 +901,6 @@ func setUpServiceTest(t *testing.T, withDashboardMock bool, cfgOverrides ...conf cfg.CloudMigration.SnapshotFolder = filepath.Join(os.TempDir(), uuid.NewString()) dashboardService := dashboards.NewFakeDashboardService(t) - if withDashboardMock { - dashboardService.On("GetAllDashboards", mock.Anything).Return( - []*dashboards.Dashboard{ - { - UID: "1", - Data: simplejson.New(), - }, - }, - nil, - ) - } dsService := &datafakes.FakeDataSourceService{ DataSources: []*datasources.DataSource{ diff --git a/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go b/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go index b382035c150..151d55562f9 100644 --- a/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go +++ b/pkg/services/cloudmigration/cloudmigrationimpl/snapshot_mgmt_alerts_test.go @@ -45,7 +45,7 @@ func TestGetAlertMuteTimings(t *testing.T) { t.Run("it returns the mute timings", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) s.features = featuremgmt.WithFeatures(featuremgmt.FlagOnPremToCloudMigrations) user := &user.SignedInUser{OrgID: 1} @@ -69,7 +69,7 @@ func TestGetNotificationTemplates(t *testing.T) { t.Run("it returns the notification templates", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -92,7 +92,7 @@ func TestGetContactPoints(t *testing.T) { t.Run("it returns the contact points", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{ OrgID: 1, @@ -115,7 +115,7 @@ func TestGetContactPoints(t *testing.T) { t.Run("it returns an error when user lacks permission to read contact point secrets", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{ OrgID: 1, @@ -144,7 +144,7 @@ func TestGetNotificationPolicies(t *testing.T) { t.Run("it returns the contact points", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{OrgID: 1} @@ -172,7 +172,7 @@ func TestGetAlertRules(t *testing.T) { t.Run("it returns the alert rules", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: alertRulesPermissions}} @@ -191,7 +191,7 @@ func TestGetAlertRules(t *testing.T) { c.CloudMigration.AlertRulesState = setting.GMSAlertRulesPaused } - s := setUpServiceTest(t, false, alertRulesState).(*Service) + s := setUpServiceTest(t, alertRulesState).(*Service) user := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: alertRulesPermissions}} @@ -218,7 +218,7 @@ func TestGetAlertRuleGroups(t *testing.T) { t.Run("it returns the alert rule groups", func(t *testing.T) { t.Parallel() - s := setUpServiceTest(t, false).(*Service) + s := setUpServiceTest(t).(*Service) user := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: alertRulesPermissions}} @@ -257,7 +257,7 @@ func TestGetAlertRuleGroups(t *testing.T) { c.CloudMigration.AlertRulesState = setting.GMSAlertRulesPaused } - s := setUpServiceTest(t, false, alertRulesState).(*Service) + s := setUpServiceTest(t, alertRulesState).(*Service) user := &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: alertRulesPermissions}}