From 3c9ecdec8ed4df76bfa35d89d615a1ad62ea384a Mon Sep 17 00:00:00 2001 From: jschaul Date: Wed, 20 Oct 2021 13:00:50 +0200 Subject: [PATCH 1/5] updatePhone deflake debugging information This is about https://wearezeta.atlassian.net/browse/BE-526 I think what's happening is that one test that tests the phone blocking adds a record into the brig.excluded_phones entry. Then, another, unrelated test, if unlucky enough to randomly generate a phone number contained under that prefix, fails in the PUT /self/phone call. * 1) update integration test output to give better information and link to a flaky test description * 2) change the code to (hopefully) avoid this flake to re-occur. The changes to integration tests will lead to the following output on failure: user account put /i/users/:uid/sso-id: Exception: Assertions failed: 1: 202 =/= 403 2: updatePhone (PUT /self/phone): failed to update to Phone {fromPhone = "+046965171332989"} - might be a flaky test tracked in https://wearezeta.atlassian.net/browse/BE-526 Response was: Response {responseStatus = Status {statusCode = 403, statusMessage = "Forbidden"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Wed, 20 Oct 2021 14:41:27 GMT"),("Server","Warp/3.3.13"),("Content-Encoding","gzip"),("Content-Type","application/json")], responseBody = Just "{\"code\":403,\"message\":\"The given phone number has been blacklisted due to suspected abuse or a complaint.\",\"label\":\"blacklisted-phone\"}", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose} CallStack (from HasCallStack): error, called at src/Bilge/Assert.hs:89:5 in bilge-0.22.0-5tCtgpJGKRb38JsbN4shGd:Bilge.Assert UserId -> Phone -> Http () +updatePhone :: HasCallStack => Brig -> UserId -> Phone -> Http () updatePhone brig uid phn = do -- update phone let phoneUpdate = RequestBodyLBS . encode $ PhoneUpdate phn - put (brig . path "/self/phone" . contentJson . zUser uid . zConn "c" . body phoneUpdate) - !!! (const 202 === statusCode) + failMsg = "updatePhone (PUT /self/phone): failed to update to " <> show phn <> " - might be a flaky test tracked in https://wearezeta.atlassian.net/browse/BE-526" + put (brig . path "/self/phone" . contentJson . zUser uid . zConn "c" . body phoneUpdate) !!! do + const 202 === statusCode + assertTrue failMsg ((== 202) . statusCode) -- activate act <- getActivationCode brig (Right phn) case act of From 40e85eb6d939ca8c821f83b37e4af139d0ed23a8 Mon Sep 17 00:00:00 2001 From: jschaul Date: Wed, 20 Oct 2021 23:34:12 +0200 Subject: [PATCH 2/5] undo changes in src as they make another test fail --- services/brig/src/Brig/API/User.hs | 9 +++------ services/brig/src/Brig/Phone.hs | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/services/brig/src/Brig/API/User.hs b/services/brig/src/Brig/API/User.hs index 9d907d4615..cf8e924701 100644 --- a/services/brig/src/Brig/API/User.hs +++ b/services/brig/src/Brig/API/User.hs @@ -112,7 +112,6 @@ import qualified Brig.IO.Intra as Intra import qualified Brig.InternalEvent.Types as Internal import Brig.Options hiding (Timeout, internalEvents) import Brig.Password -import Brig.Phone (isTestPhone) import qualified Brig.Queue as Queue import qualified Brig.Team.DB as Team import Brig.Types @@ -625,11 +624,9 @@ changePhone u phone = do when blacklisted $ throwE (BlacklistedNewPhone canonical) -- check if any prefixes of this phone number are blocked - -- (but don't do this check during tests with test phone numbers prefixed with +0) - unless (isTestPhone (fromPhone canonical)) $ do - prefixExcluded <- lift $ Blacklist.existsAnyPrefix canonical - when prefixExcluded $ - throwE (BlacklistedNewPhone canonical) + prefixExcluded <- lift $ Blacklist.existsAnyPrefix canonical + when prefixExcluded $ + throwE (BlacklistedNewPhone canonical) act <- lift $ Data.newActivation pk timeout (Just u) return (act, canonical) diff --git a/services/brig/src/Brig/Phone.hs b/services/brig/src/Brig/Phone.hs index b05c832d12..02379acb2b 100644 --- a/services/brig/src/Brig/Phone.hs +++ b/services/brig/src/Brig/Phone.hs @@ -22,7 +22,6 @@ module Brig.Phone PhoneException (..), sendCall, sendSms, - isTestPhone, -- * Validation validatePhone, From 395ad53e63e8dfa5f36308f1abfea48faefa9a22 Mon Sep 17 00:00:00 2001 From: jschaul Date: Wed, 20 Oct 2021 23:40:26 +0200 Subject: [PATCH 3/5] Add a cleanup line --- services/brig/test/integration/API/User/Account.hs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/brig/test/integration/API/User/Account.hs b/services/brig/test/integration/API/User/Account.hs index a4b9646b15..3eab7e202e 100644 --- a/services/brig/test/integration/API/User/Account.hs +++ b/services/brig/test/integration/API/User/Account.hs @@ -780,6 +780,9 @@ testPhoneUpdateBlacklisted brig = do const 200 === statusCode const (Right Nothing) === fmap userPhone . responseJsonEither + -- cleanup to avoid other tests failing sporadically + deletePrefix brig prefix + testCreateAccountPendingActivationKey :: Opt.Opts -> Brig -> Http () testCreateAccountPendingActivationKey (Opt.setRestrictUserCreation . Opt.optSettings -> Just True) _ = pure () testCreateAccountPendingActivationKey _ brig = do From c90ff54dd23ac1c2705ccfda77cb08876fcc4c52 Mon Sep 17 00:00:00 2001 From: jschaul Date: Thu, 21 Oct 2021 12:45:09 +0200 Subject: [PATCH 4/5] fixup --- services/brig/test/integration/API/User/Account.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/brig/test/integration/API/User/Account.hs b/services/brig/test/integration/API/User/Account.hs index 3eab7e202e..15b9804140 100644 --- a/services/brig/test/integration/API/User/Account.hs +++ b/services/brig/test/integration/API/User/Account.hs @@ -781,7 +781,7 @@ testPhoneUpdateBlacklisted brig = do const (Right Nothing) === fmap userPhone . responseJsonEither -- cleanup to avoid other tests failing sporadically - deletePrefix brig prefix + deletePrefix brig (phonePrefix prefix) testCreateAccountPendingActivationKey :: Opt.Opts -> Brig -> Http () testCreateAccountPendingActivationKey (Opt.setRestrictUserCreation . Opt.optSettings -> Just True) _ = pure () From dc978188884c7c6c88aaa64dca3568dd26b52430 Mon Sep 17 00:00:00 2001 From: jschaul Date: Thu, 21 Oct 2021 14:26:33 +0200 Subject: [PATCH 5/5] Hi CI