From 267d4c0fe2e71589c26d53ca6c285b7e64c29dc0 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 21 Sep 2022 12:36:14 +0200 Subject: [PATCH 1/5] Add info on very rare race condition to integration test. --- .../test/integration/API/Teams/Feature.hs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/services/galley/test/integration/API/Teams/Feature.hs b/services/galley/test/integration/API/Teams/Feature.hs index 365584c46f5..c958d5143e8 100644 --- a/services/galley/test/integration/API/Teams/Feature.hs +++ b/services/galley/test/integration/API/Teams/Feature.hs @@ -552,6 +552,27 @@ testSimpleFlagTTLOverride defaultValue ttl ttlAfter = do setFlagInternal otherValue ttl getFlag otherValue getFeatureConfig otherValue ttl + {- + *very* rarely, the above line trips over a timing issue: + + TTL / Overrides + increase to unlimited: FAIL (0.39s) + Error message: expected: FeatureTTLSeconds 2 + but got: FeatureTTLSeconds 1 + + CallStack (from HasCallStack): + assertFailure, called at ./Test/Tasty/HUnit/Orig.hs:68:32 in tasty-hunit-0.10.0.2-7a8e82b312c39043182b210e48b6d2c04e709dd607f742efc62afac8e985fd23:Test.Tasty.HUnit.Orig + assertEqual, called at ./Test/Tasty/HUnit/Orig.hs:90:23 in tasty-hunit-0.10.0.2-7a8e82b312c39043182b210e48b6d2c04e709dd607f742efc62afac8e985fd23:Test.Tasty.HUnit.Orig + @?=, called at test/integration/API/Teams/Feature.hs:505:18 in main:API.Teams.Feature + getFeatureConfig, called at test/integration/API/Teams/Feature.hs:554:3 in main:API.Teams.Feature + testSimpleFlagTTLOverride, called at test/integration/API/Teams/Feature.hs:95:44 in main:API.Teams.Feature + Use -p '/increase to unlimited/' to rerun this test only. + + probably easy to solve (just make the checks around here tolerant to one-off errors in + one direction), but i'm not sure this is worth our time? + + -} + getFlagInternal otherValue case (ttl, ttlAfter) of From 33f1e79579f0248c79ca29a5adeba22ed459a5e0 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Thu, 22 Sep 2022 21:26:29 +0200 Subject: [PATCH 2/5] hi ci From 093a7d78baaa4a63d051bbf5ec513af9c2ea8e67 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 23 Sep 2022 09:58:14 +0200 Subject: [PATCH 3/5] Fix another race condition --- services/brig/test/integration/API/Provider.hs | 3 ++- services/brig/test/integration/Util.hs | 4 ++++ services/galley/test/integration/API/Teams.hs | 5 +---- services/galley/test/integration/TestHelpers.hs | 5 +++++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/services/brig/test/integration/API/Provider.hs b/services/brig/test/integration/API/Provider.hs index e2caccbb30f..6b6674f748c 100644 --- a/services/brig/test/integration/API/Provider.hs +++ b/services/brig/test/integration/API/Provider.hs @@ -733,7 +733,8 @@ testDeleteTeamBotTeam config db brig galley cannon = withTestService config db b forM_ [uid1, uid2] $ \uid -> do void $ retryWhileN 20 (/= Intra.Deleted) (getStatus brig uid) chkStatus brig uid Intra.Deleted - getConversation galley uid cid !!! const 404 === statusCode + eventually $ do + getConversation galley uid cid !!! const 404 === statusCode -- Check the bot cannot see the conversation either getBotConv galley bid cid !!! const 404 === statusCode diff --git a/services/brig/test/integration/Util.hs b/services/brig/test/integration/Util.hs index 75960516505..e832bb08b12 100644 --- a/services/brig/test/integration/Util.hs +++ b/services/brig/test/integration/Util.hs @@ -1048,6 +1048,10 @@ aFewTimes (\_ -> pure . not . good) (const action) +-- see also: `aFewTimes`. we should really clean this up. +eventually :: (MonadIO m, MonadMask m) => m a -> m a +eventually = recovering (limitRetries 3 <> exponentialBackoff 100000) [] . const + assertOne :: (HasCallStack, MonadIO m, Show a) => [a] -> m a assertOne [a] = pure a assertOne xs = liftIO . assertFailure $ "Expected exactly one element, found " <> show xs diff --git a/services/galley/test/integration/API/Teams.hs b/services/galley/test/integration/API/Teams.hs index 0d25f0b8cb1..df7d9fe6f31 100644 --- a/services/galley/test/integration/API/Teams.hs +++ b/services/galley/test/integration/API/Teams.hs @@ -33,7 +33,6 @@ import qualified Brig.Types.Intra as Brig import Control.Arrow ((>>>)) import Control.Lens hiding ((#), (.=)) import Control.Monad.Catch -import Control.Retry import Data.Aeson hiding (json) import Data.ByteString.Conversion import Data.ByteString.Lazy (fromStrict) @@ -75,7 +74,7 @@ import Test.Tasty import Test.Tasty.Cannon (TimeoutUnit (..), (#)) import qualified Test.Tasty.Cannon as WS import Test.Tasty.HUnit -import TestHelpers (test, viewFederationDomain) +import TestHelpers (eventually, test, viewFederationDomain) import TestSetup (TestM, TestSetup, tsBrig, tsCannon, tsGConf, tsGalley) import UnliftIO (mapConcurrently) import Wire.API.Conversation @@ -491,8 +490,6 @@ testCreateOne2OneWithMembers (rolePermissions -> perms) = do -- | At the time of writing this test, the only event sent to this queue is 'MemberJoin'. testTeamQueue :: TestM () testTeamQueue = do - let eventually = recovering (limitRetries 3 <> exponentialBackoff 100000) [] . const - (owner, tid) <- createBindingTeam eventually $ do queue <- getTeamQueue owner Nothing Nothing False diff --git a/services/galley/test/integration/TestHelpers.hs b/services/galley/test/integration/TestHelpers.hs index e7932f789d9..88b35c4e773 100644 --- a/services/galley/test/integration/TestHelpers.hs +++ b/services/galley/test/integration/TestHelpers.hs @@ -21,6 +21,8 @@ module TestHelpers where import API.SQS import Control.Lens (view) +import Control.Monad.Catch (MonadMask) +import Control.Retry import Data.Domain (Domain) import Data.Qualified import qualified Galley.Aws as Aws @@ -60,3 +62,6 @@ qualifyLocal :: a -> TestM (Local a) qualifyLocal x = do domain <- viewFederationDomain pure $ toLocalUnsafe domain x + +eventually :: (MonadIO m, MonadMask m) => m a -> m a +eventually = recovering (limitRetries 3 <> exponentialBackoff 100000) [] . const From b4f509e8c7fd7c9945862200ad916ddbd6b07f17 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 23 Sep 2022 10:01:22 +0200 Subject: [PATCH 4/5] Better idea. --- .../test/integration/API/Teams/Feature.hs | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/services/galley/test/integration/API/Teams/Feature.hs b/services/galley/test/integration/API/Teams/Feature.hs index c958d5143e8..b2c952ef4fd 100644 --- a/services/galley/test/integration/API/Teams/Feature.hs +++ b/services/galley/test/integration/API/Teams/Feature.hs @@ -495,17 +495,17 @@ testSimpleFlagTTLOverride defaultValue ttl ttlAfter = do nonMember <- Util.randomUser let getFlag :: HasCallStack => Public.FeatureStatus -> TestM () - getFlag expected = + getFlag expected = eventually $ do flip (assertFlagNoConfig @cfg) expected $ Util.getTeamFeatureFlag @cfg member tid getFeatureConfig :: HasCallStack => Public.FeatureStatus -> FeatureTTL -> TestM () - getFeatureConfig expectedStatus expectedTtl = do + getFeatureConfig expectedStatus expectedTtl = eventually $ do actual <- Util.getFeatureConfig @cfg member liftIO $ Public.wsStatus actual @?= expectedStatus liftIO $ Public.wsTTL actual @?= expectedTtl getFlagInternal :: HasCallStack => Public.FeatureStatus -> TestM () - getFlagInternal expected = + getFlagInternal expected = eventually $ do flip (assertFlagNoConfig @cfg) expected $ Util.getTeamFeatureFlagInternal @cfg tid setFlagInternal :: Public.FeatureStatus -> FeatureTTL -> TestM () @@ -552,27 +552,6 @@ testSimpleFlagTTLOverride defaultValue ttl ttlAfter = do setFlagInternal otherValue ttl getFlag otherValue getFeatureConfig otherValue ttl - {- - *very* rarely, the above line trips over a timing issue: - - TTL / Overrides - increase to unlimited: FAIL (0.39s) - Error message: expected: FeatureTTLSeconds 2 - but got: FeatureTTLSeconds 1 - - CallStack (from HasCallStack): - assertFailure, called at ./Test/Tasty/HUnit/Orig.hs:68:32 in tasty-hunit-0.10.0.2-7a8e82b312c39043182b210e48b6d2c04e709dd607f742efc62afac8e985fd23:Test.Tasty.HUnit.Orig - assertEqual, called at ./Test/Tasty/HUnit/Orig.hs:90:23 in tasty-hunit-0.10.0.2-7a8e82b312c39043182b210e48b6d2c04e709dd607f742efc62afac8e985fd23:Test.Tasty.HUnit.Orig - @?=, called at test/integration/API/Teams/Feature.hs:505:18 in main:API.Teams.Feature - getFeatureConfig, called at test/integration/API/Teams/Feature.hs:554:3 in main:API.Teams.Feature - testSimpleFlagTTLOverride, called at test/integration/API/Teams/Feature.hs:95:44 in main:API.Teams.Feature - Use -p '/increase to unlimited/' to rerun this test only. - - probably easy to solve (just make the checks around here tolerant to one-off errors in - one direction), but i'm not sure this is worth our time? - - -} - getFlagInternal otherValue case (ttl, ttlAfter) of From 930caea651c47652007c79bfbf106f561b5d6e65 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Fri, 23 Sep 2022 10:27:47 +0200 Subject: [PATCH 5/5] ... --- services/galley/test/integration/API/Teams/Feature.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/galley/test/integration/API/Teams/Feature.hs b/services/galley/test/integration/API/Teams/Feature.hs index b2c952ef4fd..701aad80902 100644 --- a/services/galley/test/integration/API/Teams/Feature.hs +++ b/services/galley/test/integration/API/Teams/Feature.hs @@ -51,7 +51,7 @@ import Test.QuickCheck (Gen, generate, suchThat) import Test.Tasty import qualified Test.Tasty.Cannon as WS import Test.Tasty.HUnit (assertFailure, (@?=)) -import TestHelpers (test) +import TestHelpers (eventually, test) import TestSetup import Wire.API.Conversation.Protocol (ProtocolTag (ProtocolMLSTag, ProtocolProteusTag)) import qualified Wire.API.Event.FeatureConfig as FeatureConfig