From 3124b812295b52d84be7fda47c2299f7565197ae Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 11 Aug 2020 21:03:23 +0530 Subject: [PATCH] MM-27040: Fix flaky tests due to same UpdateAt timestamp (#15216) * MM-27040: Fix flaky tests due to same UpdateAt timestamp Update queries are not guaranteed to always update the row. Since we are only bumping the UpdateAt timestamps, if 2 update requests hit concurrently within the same millisecond, then one is bound to fail. We fix all cases in the codebase where we were updating the UpdateAt field to not check for count != 1, but rather count > 1. A similar fix was already done in 3f46cf6f60863a356c8994f1df306464feb8ae25. * Remove incorrect test --- store/sqlstore/bot_store.go | 2 +- store/sqlstore/channel_store.go | 4 ++-- store/sqlstore/oauth_store.go | 2 +- store/sqlstore/team_store.go | 2 +- store/sqlstore/user_store.go | 2 +- store/storetest/channel_store.go | 4 ---- 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/store/sqlstore/bot_store.go b/store/sqlstore/bot_store.go index 4a8ebf61f2e..788aad04ae7 100644 --- a/store/sqlstore/bot_store.go +++ b/store/sqlstore/bot_store.go @@ -205,7 +205,7 @@ func (us SqlBotStore) Update(bot *model.Bot) (*model.Bot, error) { if count, err := us.GetMaster().Update(botFromModel(bot)); err != nil { return nil, errors.Wrapf(err, "update: user_id=%s", bot.UserId) - } else if count != 1 { + } else if count > 1 { return nil, fmt.Errorf("unexpected count while updating bot: count=%d, userId=%s", count, bot.UserId) } diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index e7f3611a113..fb2e03fb18c 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -907,8 +907,8 @@ func (s SqlChannelStore) updateChannelT(transaction *gorp.Transaction, channel * return nil, errors.Wrapf(err, "failed to update channel with id=%s", channel.Id) } - if count != 1 { - return nil, fmt.Errorf("the expected number of channels to be updated is 1 but was %d", count) + if count > 1 { + return nil, fmt.Errorf("the expected number of channels to be updated is <=1 but was %d", count) } return channel, nil diff --git a/store/sqlstore/oauth_store.go b/store/sqlstore/oauth_store.go index f1a4a353b4b..4f262167acb 100644 --- a/store/sqlstore/oauth_store.go +++ b/store/sqlstore/oauth_store.go @@ -100,7 +100,7 @@ func (as SqlOAuthStore) UpdateApp(app *model.OAuthApp) (*model.OAuthApp, error) if err != nil { return nil, errors.Wrapf(err, "failed to update OAuthApp with id=%s", app.Id) } - if count != 1 { + if count > 1 { return nil, store.NewErrInvalidInput("OAuthApp", "Id", app.Id) } return app, nil diff --git a/store/sqlstore/team_store.go b/store/sqlstore/team_store.go index 41e8bbeb100..23019603096 100644 --- a/store/sqlstore/team_store.go +++ b/store/sqlstore/team_store.go @@ -296,7 +296,7 @@ func (s SqlTeamStore) Update(team *model.Team) (*model.Team, *model.AppError) { if err != nil { return nil, model.NewAppError("SqlTeamStore.Update", "store.sql_team.update.updating.app_error", nil, "id="+team.Id+", "+err.Error(), http.StatusInternalServerError) } - if count != 1 { + if count > 1 { return nil, model.NewAppError("SqlTeamStore.Update", "store.sql_team.update.app_error", nil, "id="+team.Id, http.StatusInternalServerError) } diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 56a0e6edd05..9411dcbd816 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -215,7 +215,7 @@ func (us SqlUserStore) Update(user *model.User, trustedUpdateData bool) (*model. return nil, model.NewAppError("SqlUserStore.Update", "store.sql_user.update.updating.app_error", nil, "user_id="+user.Id+", "+err.Error(), http.StatusInternalServerError) } - if count != 1 { + if count > 1 { return nil, model.NewAppError("SqlUserStore.Update", "store.sql_user.update.app_error", nil, fmt.Sprintf("user_id=%v, count=%v", user.Id, count), http.StatusInternalServerError) } diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 4114b29c22e..db7d1dadc8a 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -304,10 +304,6 @@ func testChannelStoreUpdate(t *testing.T, ss store.Store) { _, err = ss.Channel().Update(&o1) require.NotNil(t, err, "Update should have failed because of missing key") - o1.Id = model.NewId() - _, err = ss.Channel().Update(&o1) - require.NotNil(t, err, "update should have failed because id change") - o2.Name = o1.Name _, err = ss.Channel().Update(&o2) require.NotNil(t, err, "update should have failed because of existing name")