From 102a2748d79d604d25cf91233054cefc1e1639bf Mon Sep 17 00:00:00 2001 From: Akshay Mankar Date: Wed, 18 Dec 2024 09:51:21 +0100 Subject: [PATCH 1/4] integration/testConsumeTempEvents: Remove race causing flaky test (#4372) The creation of temp queue races with client creation event. This is removed by first expecting the event to be served to the real client and then listening to events temporarily --- integration/test/Test/Events.hs | 45 +++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/integration/test/Test/Events.hs b/integration/test/Test/Events.hs index 97aeb660fb6..6edfc70d0db 100644 --- a/integration/test/Test/Events.hs +++ b/integration/test/Test/Events.hs @@ -67,28 +67,41 @@ testConsumeTempEvents = do alice <- randomUser OwnDomain def client0 <- addClient alice def {acapabilities = Just ["consumable-notifications"]} >>= getJSON 201 - runCodensity (createEventsWebSocket alice Nothing) $ \ws -> do - clientId <- objId client0 + clientId0 <- objId client0 - void $ assertEvent ws $ \e -> do - e %. "type" `shouldMatch` "event" - e %. "data.event.payload.0.type" `shouldMatch` "user.client-add" - e %. "data.event.payload.0.client.id" `shouldMatch` clientId + lowerCodensity $ do + ws0 <- createEventsWebSocket alice (Just clientId0) - ackEvent ws e + -- Ensure there is no race between event for this client being pushed and temp + -- consumer being created + lift $ do + expectAndAckNewClientEvent ws0 clientId0 + assertNoEvent_ ws0 - runCodensity (createEventsWebSocket alice Nothing) $ \ws -> do - client <- addClient alice def {acapabilities = Just ["consumable-notifications"]} >>= getJSON 201 - clientId <- objId client + wsTemp <- createEventsWebSocket alice Nothing - void $ assertEvent ws $ \e -> do - e %. "type" `shouldMatch` "event" - e %. "data.event.payload.0.type" `shouldMatch` "user.client-add" - e %. "data.event.payload.0.client.id" `shouldMatch` clientId + lift $ do + client1 <- addClient alice def {acapabilities = Just ["consumable-notifications"]} >>= getJSON 201 + clientId1 <- objId client1 - ackEvent ws e + -- Temp client gets this event as it happens after temp client has started + -- listening + void $ expectAndAckNewClientEvent wsTemp clientId1 - assertNoEvent_ ws + -- Client0 should also be notified even if there is a temp client + void $ expectAndAckNewClientEvent ws0 clientId1 + + assertNoEvent_ wsTemp + assertNoEvent_ ws0 + where + expectAndAckNewClientEvent :: EventWebSocket -> String -> App () + expectAndAckNewClientEvent ws cid = + assertEvent ws $ \e -> do + e %. "type" `shouldMatch` "event" + e %. "data.event.payload.0.type" `shouldMatch` "user.client-add" + e %. "data.event.payload.0.client.id" `shouldMatch` cid + + ackEvent ws e testMLSTempEvents :: (HasCallStack) => App () testMLSTempEvents = do From a9e53f46a58ead16a952aa89b80f038add3ee5ed Mon Sep 17 00:00:00 2001 From: Sven Tennie Date: Wed, 18 Dec 2024 10:07:55 +0100 Subject: [PATCH 2/4] Format submodule with existing Ormolu script (#4377) * Format submodule with existing Ormolu script The wrapper calls the renamed old script to also format the wire-server-enterprise submodule. * Add changelog * Upgrade wire-server-enterprise submodule To fix formatting issues. --- changelog.d/5-internal/submodule-formatting | 2 + services/wire-server-enterprise | 2 +- tools/ormolu.sh | 108 ++---------------- tools/ormolu1.sh | 118 ++++++++++++++++++++ 4 files changed, 128 insertions(+), 102 deletions(-) create mode 100644 changelog.d/5-internal/submodule-formatting create mode 100755 tools/ormolu1.sh diff --git a/changelog.d/5-internal/submodule-formatting b/changelog.d/5-internal/submodule-formatting new file mode 100644 index 00000000000..31ab2d3697e --- /dev/null +++ b/changelog.d/5-internal/submodule-formatting @@ -0,0 +1,2 @@ +Adjust the existing Ormolu script to format the wire-server-enterprise submodule +as well. diff --git a/services/wire-server-enterprise b/services/wire-server-enterprise index 61560248714..126abff4608 160000 --- a/services/wire-server-enterprise +++ b/services/wire-server-enterprise @@ -1 +1 @@ -Subproject commit 615602487147e7ac4fae49e72b661ace437e8ce6 +Subproject commit 126abff46082573c958aed8e6a338a00c608c376 diff --git a/tools/ormolu.sh b/tools/ormolu.sh index 7058e51517b..d063fa77361 100755 --- a/tools/ormolu.sh +++ b/tools/ormolu.sh @@ -1,106 +1,12 @@ #!/usr/bin/env bash +# This is a wrapper script to call tools/ormolu1.sh It calls that script for +# both, wire-server and the services/wire-server-enterprise submodule. It should +# be compatible to the old behaviour, with the only addition, that the submodule +# is visited as well. set -e -cd "$( dirname "${BASH_SOURCE[0]}" )/.." +cd "$(dirname "${BASH_SOURCE[0]}")" -ALLOW_DIRTY_WC="0" -ARG_ORMOLU_MODE="inplace" -PR_BASE=${PR_BASE:-"origin/develop"} - -USAGE=" -This bash script can either - -* apply ormolu formatting in-place (the default) - -* check all modules for formatting and fail if ormolu needs to be applied (-c flag). This can be run in CI to make sure no branches with non-ormolu formatting make get merged. - -USAGE: $0 - -h : show this help. - -f pr : run even if working copy is dirty. Check only on files changed by branch. - -f all : run even if working copy is dirty. Check all files. - -c : set ormolu mode to 'check'. default: 'inplace' -" - -usage() { echo "$USAGE" 1>&2; exit 1; } - -# Option parsing: -# https://sookocheff.com/post/bash/parsing-bash-script-arguments-with-shopts/ -while getopts ":f:ch" opt; do - case ${opt} in - f ) f=${OPTARG} - if [ "$f" = "pr" ]; then - ALLOW_DIRTY_WC=1 - elif [ "$f" = "all" ]; then - ALLOW_DIRTY_WC=1 - else - usage - fi - ;; - c ) ARG_ORMOLU_MODE="check" - ;; - h ) echo "$USAGE" 1>&2 - exit 0 - ;; - *) usage;; - esac -done -shift $((OPTIND -1)) - -if [ "$#" -ne 0 ]; then - echo "$USAGE" 1>&2 - exit 1 -fi - -if [ "$(git status -s | grep -v \?\?)" != "" ]; then - if [ "$ALLOW_DIRTY_WC" == "1" ]; then - : - else - echo "Working copy is not clean." - echo "Run with -f pr or -f all if you want to run ormolu anyway" - exit 1 - fi -fi - -echo "ormolu mode: $ARG_ORMOLU_MODE" -echo "language extensions are taken from the resp. cabal files" - -FAILURES=0 - -if [ -t 1 ]; then - : "${ORMOLU_CONDENSE_OUTPUT:=1}" -fi - -if [ "$f" = "all" ] || [ "$f" = "" ]; then - files=$(git ls-files | grep '\.hsc\?$') -elif [ "$f" = "pr" ]; then - files=$(git diff --diff-filter=ACMR --name-only "$PR_BASE"... | { grep '\.hsc\?$' || true; }; git diff --diff-filter=ACMR --name-only HEAD | { grep \.hs\$ || true ; }) -fi - -count=$( echo "$files" | sed '/^\s*$/d' | wc -l ) -echo "Checking $count file(s)…" - -for hsfile in $files; do - # run in background so that we can detect Ctrl-C properly - ormolu --mode $ARG_ORMOLU_MODE --check-idempotence "$hsfile" & - wait $! && err=0 || err=$? - - if [ "$err" == "100" ]; then - ((++FAILURES)) - echo "$hsfile... *** FAILED" - clear="" - elif [ "$err" == "0" ]; then - echo -e "$clear$hsfile... ok" - [ "$ORMOLU_CONDENSE_OUTPUT" == "1" ] && clear="\033[A\r\033[K" - else - exit "$err" - fi -done - -if [ "$FAILURES" != 0 ]; then - echo "ormolu failed on $FAILURES files." - if [ "$ARG_ORMOLU_MODE" == "check" ]; then - echo -en "\n\nyou can fix this by running 'make format' from the git repo root.\n\n" - fi - exit 1 -fi +REL_PATH=services/wire-server-enterprise PR_BASE=origin/main ./ormolu1.sh "$@" +PR_BASE=origin/develop ./ormolu1.sh "$@" diff --git a/tools/ormolu1.sh b/tools/ormolu1.sh new file mode 100755 index 00000000000..1210c58672a --- /dev/null +++ b/tools/ormolu1.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash + +set -e + +REL_PATH=${REL_PATH:-"."} +cd "$(dirname "${BASH_SOURCE[0]}")/../$REL_PATH" + +echo Formatting branch "$PR_BASE" in "${REL_PATH}" + +ALLOW_DIRTY_WC="0" +ARG_ORMOLU_MODE="inplace" +PR_BASE=${PR_BASE:-"origin/develop"} + +USAGE=" +This bash script can either + +* apply ormolu formatting in-place (the default) + +* check all modules for formatting and fail if ormolu needs to be applied (-c flag). This can be run in CI to make sure no branches with non-ormolu formatting make get merged. + +USAGE: $0 + -h : show this help. + -f pr : run even if working copy is dirty. Check only on files changed by branch. + -f all : run even if working copy is dirty. Check all files. + -c : set ormolu mode to 'check'. default: 'inplace' +" + +usage() { + echo "$USAGE" 1>&2 + exit 1 +} + +# Option parsing: +# https://sookocheff.com/post/bash/parsing-bash-script-arguments-with-shopts/ +while getopts ":f:ch" opt; do + case ${opt} in + f) + f=${OPTARG} + if [ "$f" = "pr" ]; then + ALLOW_DIRTY_WC=1 + elif [ "$f" = "all" ]; then + ALLOW_DIRTY_WC=1 + else + usage + fi + ;; + c) + ARG_ORMOLU_MODE="check" + ;; + h) + echo "$USAGE" 1>&2 + exit 0 + ;; + *) usage ;; + esac +done +shift $((OPTIND - 1)) + +if [ "$#" -ne 0 ]; then + echo "$USAGE" 1>&2 + exit 1 +fi + +if [ "$(git status -s | grep -v \?\?)" != "" ]; then + if [ "$ALLOW_DIRTY_WC" == "1" ]; then + : + else + echo "Working copy is not clean." + echo "Run with -f pr or -f all if you want to run ormolu anyway" + exit 1 + fi +fi + +echo "ormolu mode: $ARG_ORMOLU_MODE" +echo "language extensions are taken from the resp. cabal files" + +FAILURES=0 + +if [ -t 1 ]; then + : "${ORMOLU_CONDENSE_OUTPUT:=1}" +fi + +if [ "$f" = "all" ] || [ "$f" = "" ]; then + files=$(git ls-files | grep '\.hsc\?$') +elif [ "$f" = "pr" ]; then + files=$( + git diff --diff-filter=ACMR --name-only "$PR_BASE"... | { grep '\.hsc\?$' || true; } + git diff --diff-filter=ACMR --name-only HEAD | { grep \.hs\$ || true; } + ) +fi + +count=$(echo "$files" | sed '/^\s*$/d' | wc -l) +echo "Checking $count file(s)…" + +for hsfile in $files; do + # run in background so that we can detect Ctrl-C properly + ormolu --mode $ARG_ORMOLU_MODE --check-idempotence "$hsfile" & + wait $! && err=0 || err=$? + + if [ "$err" == "100" ]; then + ((++FAILURES)) + echo "$hsfile... *** FAILED" + clear="" + elif [ "$err" == "0" ]; then + echo -e "$clear$hsfile... ok" + [ "$ORMOLU_CONDENSE_OUTPUT" == "1" ] && clear="\033[A\r\033[K" + else + exit "$err" + fi +done + +if [ "$FAILURES" != 0 ]; then + echo "ormolu failed on $FAILURES files." + if [ "$ARG_ORMOLU_MODE" == "check" ]; then + echo -en "\n\nyou can fix this by running 'make format' from the git repo root.\n\n" + fi + exit 1 +fi From 8bf04be7c40aa6bb753cce15ba0b1c33fc00a229 Mon Sep 17 00:00:00 2001 From: Matthias Fischmann Date: Wed, 18 Dec 2024 16:03:03 +0100 Subject: [PATCH 3/4] galley: Remove unused IntraComponent Gundeck (#4381) --- services/galley/src/Galley/Intra/Util.hs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/services/galley/src/Galley/Intra/Util.hs b/services/galley/src/Galley/Intra/Util.hs index fa24f814ae5..18070288aca 100644 --- a/services/galley/src/Galley/Intra/Util.hs +++ b/services/galley/src/Galley/Intra/Util.hs @@ -36,15 +36,13 @@ import Galley.Env hiding (brig) import Galley.Monad import Galley.Options import Imports hiding (log) -import Network.HTTP.Types -data IntraComponent = Brig | Spar | Gundeck +data IntraComponent = Brig | Spar deriving (Show) componentName :: IntraComponent -> String componentName Brig = "brig" componentName Spar = "spar" -componentName Gundeck = "gundeck" componentRequest :: IntraComponent -> Opts -> Request -> Request componentRequest Brig o = @@ -53,17 +51,10 @@ componentRequest Brig o = componentRequest Spar o = B.host (encodeUtf8 o._spar.host) . B.port (portNumber $ fromIntegral . port $ o._spar) -componentRequest Gundeck o = - B.host (encodeUtf8 o._gundeck.host) - . B.port (portNumber $ fromIntegral . port $ o._gundeck) - . method POST - . path "/i/push/v2" - . expect2xx componentRetryPolicy :: IntraComponent -> RetryPolicy componentRetryPolicy Brig = x1 componentRetryPolicy Spar = x1 -componentRetryPolicy Gundeck = limitRetries 0 call :: IntraComponent -> From 34f8e17a5f59706879f675fdae001ff597c26063 Mon Sep 17 00:00:00 2001 From: Akshay Mankar Date: Wed, 18 Dec 2024 20:18:46 +0100 Subject: [PATCH 4/4] integration: Print name of the test in logs from dynamic backends (#4383) --- integration/test/Testlib/Env.hs | 7 ++++--- integration/test/Testlib/ModService.hs | 12 ++++++------ integration/test/Testlib/Run.hs | 8 ++++---- integration/test/Testlib/RunServices.hs | 2 +- integration/test/Testlib/Types.hs | 3 ++- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/integration/test/Testlib/Env.hs b/integration/test/Testlib/Env.hs index 401332e4a0b..d14cb630a17 100644 --- a/integration/test/Testlib/Env.hs +++ b/integration/test/Testlib/Env.hs @@ -130,8 +130,8 @@ mkGlobalEnv cfgFile = do pure $ Just sslContext createSSLContext Nothing = pure Nothing -mkEnv :: GlobalEnv -> Codensity IO Env -mkEnv ge = do +mkEnv :: Maybe String -> GlobalEnv -> Codensity IO Env +mkEnv currentTestName ge = do mls <- liftIO . newIORef =<< mkMLSState liftIO $ do pks <- newIORef (zip [1 ..] somePrekeys) @@ -161,7 +161,8 @@ mkEnv ge = do mls = mls, resourcePool = ge.gBackendResourcePool, rabbitMQConfig = ge.gRabbitMQConfig, - timeOutSeconds = ge.gTimeOutSeconds + timeOutSeconds = ge.gTimeOutSeconds, + currentTestName } allCiphersuites :: [Ciphersuite] diff --git a/integration/test/Testlib/ModService.hs b/integration/test/Testlib/ModService.hs index a41244faaae..31716b94e9c 100644 --- a/integration/test/Testlib/ModService.hs +++ b/integration/test/Testlib/ModService.hs @@ -358,17 +358,17 @@ withProcess :: (HasCallStack) => BackendResource -> ServiceOverrides -> Service withProcess resource overrides service = do let domain = berDomain resource sm <- lift $ getServiceMap domain + env <- lift ask getConfig <- lift . appToIO $ readServiceConfig service >>= updateServiceMapInConfig resource service >>= lookupConfigOverride overrides service let execName = configName service - (cwd, exe) <- - lift $ asks \env -> case env.servicesCwdBase of - Nothing -> (Nothing, execName) - Just dir -> - (Just (dir execName), "../../dist" execName) + let (cwd, exe) = case env.servicesCwdBase of + Nothing -> (Nothing, execName) + Just dir -> + (Just (dir execName), "../../dist" execName) startNginzLocalIO <- lift $ appToIO $ startNginzLocal resource @@ -379,7 +379,7 @@ withProcess resource overrides service = do config <- getConfig tempFile <- writeTempFile "/tmp" (execName <> "-" <> domain <> "-" <> ".yaml") (cs $ Yaml.encode config) (_, Just stdoutHdl, Just stderrHdl, ph) <- createProcess (proc exe ["-c", tempFile]) {cwd = cwd, std_out = CreatePipe, std_err = CreatePipe} - let prefix = "[" <> execName <> "@" <> domain <> "] " + let prefix = "[" <> execName <> "@" <> domain <> maybe "" (":" <>) env.currentTestName <> "] " let colorize = fromMaybe id (lookup execName processColors) void $ forkIO $ logToConsole colorize prefix stdoutHdl void $ forkIO $ logToConsole colorize prefix stderrHdl diff --git a/integration/test/Testlib/Run.hs b/integration/test/Testlib/Run.hs index d7d533060c7..56caebb5055 100644 --- a/integration/test/Testlib/Run.hs +++ b/integration/test/Testlib/Run.hs @@ -31,9 +31,9 @@ import Text.Printf import UnliftIO.Async import Prelude -runTest :: GlobalEnv -> App a -> IO (Either String a) -runTest ge action = lowerCodensity $ do - env <- mkEnv ge +runTest :: String -> GlobalEnv -> App a -> IO (Either String a) +runTest testName ge action = lowerCodensity $ do + env <- mkEnv (Just testName) ge liftIO $ (Right <$> runAppWithEnv env action) `E.catches` [ E.Handler $ \(e :: SomeAsyncException) -> do @@ -121,7 +121,7 @@ runTests tests mXMLOutput cfg = do withAsync displayOutput $ \displayThread -> do -- Currently 4 seems to be stable, more seems to create more timeouts. report <- fmap mconcat $ pooledForConcurrentlyN 4 tests $ \(qname, _, _, action) -> do - (mErr, tm) <- withTime (runTest genv action) + (mErr, tm) <- withTime (runTest qname genv action) case mErr of Left err -> do writeOutput $ diff --git a/integration/test/Testlib/RunServices.hs b/integration/test/Testlib/RunServices.hs index a88686b2979..434cfbf50ef 100644 --- a/integration/test/Testlib/RunServices.hs +++ b/integration/test/Testlib/RunServices.hs @@ -71,7 +71,7 @@ main = do (_, _, _, ph) <- createProcess cp exitWith =<< waitForProcess ph - runCodensity (mkGlobalEnv cfg >>= mkEnv) $ \env -> + runCodensity (mkGlobalEnv cfg >>= mkEnv Nothing) $ \env -> runAppWithEnv env $ lowerCodensity $ do diff --git a/integration/test/Testlib/Types.hs b/integration/test/Testlib/Types.hs index d5c95021afa..bdb732c6831 100644 --- a/integration/test/Testlib/Types.hs +++ b/integration/test/Testlib/Types.hs @@ -228,7 +228,8 @@ data Env = Env mls :: IORef MLSState, resourcePool :: ResourcePool BackendResource, rabbitMQConfig :: RabbitMQConfig, - timeOutSeconds :: Int + timeOutSeconds :: Int, + currentTestName :: Maybe String } data Response = Response