From 09ea32a03c8b2b5e6f44769bcdb7d47dc56f82e0 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 21 May 2025 08:48:46 +0530 Subject: [PATCH] MM-63300: Fix flaky test TestBusySet (#31116) (*Busy).Set would set its own timer, and additionally send a message across the cluster. In this case, the cluster is mocked locally. But the timer calculation happens again. We marshal the expiry time with b.expires.Unix() and send that as part of model.ServerBusyState. This is parsed again in ClusterEventChanged and converted to duration with time.Until. Therefore, if it takes longer for the code to reach those lines, then the new time calculated would have already expired, failing the test. To fix this, we increase the timeout. This slows down the test at the cost of extra reliability. This is a common failure point with any timer related tests. Additionally, we also change the condition in compareBusyState to check for less-than rather than strict equality. https://mattermost.atlassian.net/browse/MM-63300 ```release-note NONE ``` Co-authored-by: Mattermost Build --- server/channels/app/busy_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/server/channels/app/busy_test.go b/server/channels/app/busy_test.go index 741b684b0e8..980b32b4451 100644 --- a/server/channels/app/busy_test.go +++ b/server/channels/app/busy_test.go @@ -16,7 +16,6 @@ import ( ) func TestBusySet(t *testing.T) { - t.Skip("https://mattermost.atlassian.net/browse/MM-63300") cluster := &ClusterMock{Busy: &Busy{}, t: t} busy := NewBusy(cluster) @@ -26,11 +25,11 @@ func TestBusySet(t *testing.T) { require.False(t, busy.IsBusy()) - busy.Set(time.Millisecond * 500) + busy.Set(time.Second * 5) require.True(t, busy.IsBusy()) require.True(t, compareBusyState(t, busy, cluster.Busy)) - // should automatically expire after 500ms. + // should automatically expire after 5s. require.Eventually(t, isNotBusy, time.Second*15, time.Millisecond*100) // allow a moment for cluster to sync. require.Eventually(t, func() bool { return compareBusyState(t, busy, cluster.Busy) }, time.Second*15, time.Millisecond*20) @@ -105,13 +104,16 @@ func compareBusyState(t *testing.T, busy1 *Busy, busy2 *Busy) bool { if busy1.IsBusy() != busy2.IsBusy() { busy1JSON, _ := busy1.ToJSON() busy2JSON, _ := busy2.ToJSON() - t.Logf("busy1:%s; busy2:%s\n", busy1JSON, busy2JSON) + t.Logf("IsBusy() is not equal: busy1:%s; busy2:%s\n", busy1JSON, busy2JSON) return false } - if busy1.Expires().Unix() != busy2.Expires().Unix() { + // busy2 is the cluster expiry which could potentially be later + // than busy1 because of the recalculation of new time at ClusterEventChanged + // and then again at setWithoutNotify. + if busy1.Expires().Unix() < busy2.Expires().Unix() { busy1JSON, _ := busy1.ToJSON() busy2JSON, _ := busy2.ToJSON() - t.Logf("busy1:%s; busy2:%s\n", busy1JSON, busy2JSON) + t.Logf("busy1.Expires().Unix():%s is not less than busy2Expires().Unix():%s\n", busy1JSON, busy2JSON) return false } return true