Skip to content

Commit 3f46cf6

Browse files
agnivademattermod
andauthored
MM-18006: Fix flaky test CreateOrRestoreGroupMember (mattermost#14955)
Updating a Group or a GroupMember only changed the UpdateAt or CreateAt times respectively. And it threw an error if number of rows changed was not 1. However, it can happen that 2 calls happen so fast that 1 milisecond does not pass, or even 2 concurrent calls at the same time might happen so that model.GetMillis return the same timestamp. In those cases, the number of rows updated can be 0. The error should just check if the number is greater than 1, instead of not equal to 1. This makes it more robust and correct. Co-authored-by: Mattermod <[email protected]>
1 parent 9805a59 commit 3f46cf6

File tree

2 files changed

+8
-8
lines changed

2 files changed

+8
-8
lines changed

i18n/en.json

+4-4
Original file line numberDiff line numberDiff line change
@@ -6727,12 +6727,12 @@
67276727
"translation": "group syncable was already deleted"
67286728
},
67296729
{
6730-
"id": "store.sql_group.no_rows",
6731-
"translation": "no matching group found"
6730+
"id": "store.sql_group.more_than_one_row_changed",
6731+
"translation": "More than one row changed."
67326732
},
67336733
{
6734-
"id": "store.sql_group.no_rows_changed",
6735-
"translation": "no rows changed"
6734+
"id": "store.sql_group.no_rows",
6735+
"translation": "no matching group found"
67366736
},
67376737
{
67386738
"id": "store.sql_group.permanent_delete_members_by_user.app_error",

store/sqlstore/group_store.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,8 @@ func (s *SqlGroupStore) Update(group *model.Group) (*model.Group, *model.AppErro
232232
}
233233
return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.update_error", nil, err.Error(), http.StatusInternalServerError)
234234
}
235-
if rowsChanged != 1 {
236-
return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError)
235+
if rowsChanged > 1 {
236+
return nil, model.NewAppError("SqlGroupStore.GroupUpdate", "store.sql_group.more_than_one_row_changed", nil, "", http.StatusInternalServerError)
237237
}
238238

239239
return group, nil
@@ -427,8 +427,8 @@ func (s *SqlGroupStore) UpsertMember(groupID string, userID string) (*model.Grou
427427
if rowsChanged, err = s.GetMaster().Update(member); err != nil {
428428
return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.update_error", nil, "group_id="+member.GroupId+", user_id="+member.UserId+", "+err.Error(), http.StatusInternalServerError)
429429
}
430-
if rowsChanged != 1 {
431-
return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.no_rows_changed", nil, "", http.StatusInternalServerError)
430+
if rowsChanged > 1 {
431+
return nil, model.NewAppError("SqlGroupStore.GroupCreateOrRestoreMember", "store.sql_group.more_than_one_row_changed", nil, "", http.StatusInternalServerError)
432432
}
433433
}
434434

0 commit comments

Comments
 (0)