Merge #1029

1029: Additional fixes for integration tests on Windows  r=KtorZ a=piotr-iohk

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->


# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have updated the error message for "Connection refuse" so it is ok for Windows and Linux/Mac (hopefully)
- [x] Use 'waitForProcess' windows work-around in launcher tests & review filepath to use a platform agnostic separator
- [x] Use a temporary file as a handle for the LOGGING tests and remove this whole `collectStreams` convoluted logic :) 
- [x] remove race cond in TRANS_DELETE_05 and remove some expectation also because of this  

# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Piotr Stachyra <piotr.stachyra@iohk.io>
Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Co-authored-by: Pawel Jakubas <pawel.jakubas@iohk.io>

lib/core-integration/src/Test/Integration/Framework/DSL.hs

@@ -95,9 +95,6 @@ module Test.Integration.Framework.DSL
     , faucetUtxoAmt
     , proc'
     , waitForServer
-    , collectStreams
-    , shouldContainT
-    , shouldNotContainT
     , for
     , toQueryString
     , utcIso8601ToText
@@ -200,13 +197,11 @@ import Cardano.Wallet.Primitive.Types
 import Control.Concurrent
     ( threadDelay )
 import Control.Concurrent.Async
-    ( async, concurrently, race, wait )
-import Control.Concurrent.MVar
-    ( MVar, modifyMVar_, newMVar, takeMVar )
+    ( async, race, wait )
 import Control.Exception
-    ( SomeException (..), catch, finally, try )
+    ( SomeException (..), catch )
 import Control.Monad
-    ( forM_, join, unless, void, (>=>) )
+    ( forM_, join, unless, void )
 import Control.Monad.Catch
     ( MonadCatch )
 import Control.Monad.Fail
@@ -225,8 +220,6 @@ import Data.Foldable
     ( toList )
 import Data.Function
     ( (&) )
-import Data.Functor
-    ( (<&>) )
 import Data.Generics.Internal.VL.Lens
     ( Lens', lens, set, view, (^.) )
 import Data.Generics.Labels
@@ -255,8 +248,6 @@ import Data.Time.Text
     ( iso8601ExtendedUtc, utcTimeToText )
 import Data.Word
     ( Word64 )
-import GHC.Stack
-    ( HasCallStack )
 import GHC.TypeLits
     ( Symbol )
 import Language.Haskell.TH.Quote
@@ -276,20 +267,18 @@ import System.Directory
 import System.Exit
     ( ExitCode (..) )
 import System.IO
-    ( BufferMode (..), Handle, hClose, hFlush, hPutStr, hSetBuffering )
+    ( hClose, hFlush, hPutStr )
 import System.Process
     ( CreateProcess (..)
-    , ProcessHandle
     , StdStream (..)
     , proc
-    , terminateProcess
     , waitForProcess
     , withCreateProcess
     )
 import Test.Hspec
     ( expectationFailure )
 import Test.Hspec.Expectations.Lifted
-    ( shouldBe, shouldContain, shouldNotBe, shouldNotContain )
+    ( shouldBe, shouldContain, shouldNotBe )
 import Test.Integration.Faucet
     ( nextTxBuilder, nextWallet )
 import Test.Integration.Framework.Request
@@ -1587,63 +1576,6 @@ proc' :: FilePath -> [String] -> CreateProcess
 proc' cmd args = (proc cmd args)
     { std_in = CreatePipe, std_out = CreatePipe, std_err = CreatePipe }
 
--- | Collect lines from standard output and error streams for 65 seconds, or,
--- until a given limit is for both streams.
-collectStreams :: (Int, Int) -> CreateProcess -> IO (Text, Text)
-collectStreams (nOut0, nErr0) p = do
-    let safeP = p { std_out = CreatePipe, std_err = CreatePipe }
-    mvar <- newMVar (mempty, mempty)
-    withCreateProcess safeP $ \_ (Just o) (Just e) ph -> do
-        hSetBuffering o LineBuffering
-        hSetBuffering e LineBuffering
-        let io = race
-                (threadDelay (65 * oneSecond))
-                (collect mvar ((o, nOut0), (e, nErr0)) ph)
-        void $ io `finally` do
-            -- NOTE
-            -- Somehow, calling 'terminateProcess' isn't sufficient. We also
-            -- need to close the handles otherwise, the function resolves but
-            -- the processes remains hanging there for a while...
-            terminateProcess ph
-            flush o
-            flush e
-    takeMVar mvar
-  where
-    flush :: Handle -> IO ()
-    flush = try @SomeException . TIO.hGetContents >=> print
-
-    collect
-        :: MVar (Text, Text)
-        -> ((Handle, Int), (Handle, Int))
-        -> ProcessHandle
-        -> IO ()
-    collect mvar ((stdout, nOut), (stderr, nErr)) ph
-        | nOut <= 0 && nErr <= 0 = return ()
-        | otherwise = do
-            ((out, nOut'), (err, nErr')) <- concurrently
-                (getNextLine nOut stdout)
-                (getNextLine nErr stderr)
-            modifyMVar_ mvar (\(out0, err0) ->
-                return (out0 <> out, err0 <> err))
-            collect mvar ((stdout, nOut'), (stderr, nErr')) ph
-
-    getNextLine :: Int -> Handle -> IO (Text, Int)
-    getNextLine n h
-        | n <= 0 = return (mempty, n)
-        | otherwise = do
-            threadDelay (10 * oneMillisecond)
-            try @SomeException (TIO.hGetLine h) <&> \case
-                Left _  -> (mempty, n)
-                Right l -> (l, n-1)
-
--- | Like 'shouldContain', but with 'Text'.
-shouldContainT :: HasCallStack => Text -> Text -> IO ()
-shouldContainT a b = T.unpack a `shouldContain` T.unpack b
-
--- | Like 'shouldNotContain', but with 'Text'.
-shouldNotContainT :: HasCallStack => Text -> Text -> IO ()
-shouldNotContainT a b = T.unpack a `shouldNotContain` T.unpack b
-
 oneSecond :: Int
 oneSecond = 1_000 * oneMillisecond
 

lib/core-integration/src/Test/Integration/Scenario/CLI/Port.hs

@@ -56,58 +56,72 @@ spec = do
 
     it "PORT_01 - Can't reach server with wrong port (wallet list)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
-        (_ :: ExitCode, Stdout (_ :: String), Stderr err) <-
+        (c :: ExitCode, Stdout (_ :: String), Stderr _) <-
             listWalletsViaCLI @t ctx'
-        err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_01 - Can't reach server with wrong port (wallet create)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
         let name = "Wallet created via CLI"
         Stdout mnemonics <- generateMnemonicsViaCLI @t ["--size", "15"]
         let pwd = "Secure passphrase"
-        (_ :: ExitCode, _, err) <-
+        (c :: ExitCode, _, _) <-
             createWalletViaCLI @t ctx' [name] mnemonics "\n" pwd
-        T.unpack err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_01 - Can't reach server with wrong port (wallet get)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
         let wid = replicate 40 '0'
-        (_ :: ExitCode, Stdout (_ :: String), Stderr err) <-
+        (c :: ExitCode, Stdout (_ :: String), Stderr _) <-
             getWalletViaCLI @t ctx' wid
-        err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_01 - Can't reach server with wrong port (wallet delete)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
         let wid = replicate 40 '0'
-        (_ :: ExitCode, Stdout (_ :: String), Stderr err) <-
+        (c :: ExitCode, Stdout (_ :: String), Stderr _) <-
             deleteWalletViaCLI @t ctx' wid
-        err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_01 - Can't reach server with wrong port (wallet update)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
         let wid = replicate 40 '0'
-        (_ :: ExitCode, Stdout (_ :: String), Stderr err) <-
+        (c :: ExitCode, Stdout (_ :: String), Stderr _) <-
             updateWalletNameViaCLI @t ctx' [wid, "My Wallet"]
-        err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_01 - Can't reach server with wrong port (transction create)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
         let addr =
                 "37btjrVyb4KFjfnPUjgDKLiATLxgwBbeMAEr4vxgkq4Ea5nR6evtX99x2\
                 \QFcF8ApLM4aqCLGvhHQyRJ4JHk4zVKxNeEtTJaPCeB86LndU2YvKUTEEm"
-        (_ :: ExitCode, _, err) <-
+        (c :: ExitCode, _, _) <-
             postTransactionViaCLI @t ctx' passphrase
                 [ replicate 40 '0'
                 , "--payment", "14@" <> addr
                 ]
-        T.unpack err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_01 - Can't reach server with wrong port (address list)" $ \ctx -> do
         let ctx' = overPort @"wallet" (+1) ctx
         let wid = replicate 40 '0'
-        (_ :: ExitCode, Stdout (_ :: String), Stderr err) <-
+        (c :: ExitCode, Stdout (_ :: String), Stderr _) <-
             listAddressesViaCLI @t ctx' [wid]
-        err `shouldContain` errConnectionRefused
+        -- on Windows seems that not whole stderr is available to Stderr
+        -- hence asserting only for exit code
+        c `shouldBe` ExitFailure 1
 
     it "PORT_03 - Cannot omit --port when server uses random port (wallet list)" $ \_ -> do
         (_ :: ExitCode, Stdout (_ :: String), Stderr err) <-
@@ -201,4 +215,4 @@ passphrase :: String
 passphrase = "cardano-wallet"
 
 errConnectionRefused :: String
-errConnectionRefused = "does not exist (Connection refused)"
+errConnectionRefused = "Connection refused"

lib/core-integration/src/Test/Integration/Scenario/CLI/Transactions.hs

@@ -73,7 +73,6 @@ import Test.Integration.Framework.DSL
     , expectEventually'
     , expectValidJSON
     , faucetAmt
-    , faucetUtxoAmt
     , feeEstimator
     , fixtureWallet
     , fixtureWalletWith
@@ -137,7 +136,6 @@ spec = do
                 ( faucetAmt - feeMax - amt
                 , faucetAmt - feeMin - amt
                 )
-            , expectCliFieldEqual balanceAvailable (faucetAmt - faucetUtxoAmt)
             ]
 
         expectEventually' ctx getWalletEp balanceAvailable amt wDest
@@ -187,7 +185,6 @@ spec = do
                 ( faucetAmt - feeMax - (2*amt)
                 , faucetAmt - feeMin - (2*amt)
                 )
-            , expectCliFieldEqual balanceAvailable (faucetAmt - 2*faucetUtxoAmt)
             ]
 
         expectEventually' ctx getWalletEp balanceAvailable (2*amt) wDest
@@ -239,7 +236,6 @@ spec = do
                 ( faucetAmt - feeMax - (2*amt)
                 , faucetAmt - feeMin - (2*amt)
                 )
-            , expectCliFieldEqual balanceAvailable (faucetAmt - 2*faucetUtxoAmt)
             ]
 
         forM_ [wDest1, wDest2] $ \wDest -> do
@@ -892,30 +888,24 @@ spec = do
         let wSrcId = T.unpack (wSrc ^. walletId)
 
         -- post transaction
-        txJson <- postTxViaCLI ctx wSrc wDest 1
-        verify txJson
-            [ expectCliFieldEqual direction Outgoing
-            , expectCliFieldEqual status Pending
-            ]
-        let txId = getTxId txJson
-
-        -- verify balance on src wallet
-        (fromStdout <$> getWalletViaCLI @t ctx wSrcId)
-            >>= expectValidJSON (Proxy @ApiWallet)
-            >>= flip verify
-                [ expectCliFieldEqual balanceAvailable (faucetAmt - faucetUtxoAmt)
+        eventually_ $ do
+            txJson <- postTxViaCLI ctx wSrc wDest 1
+            verify txJson
+                [ expectCliFieldEqual direction Outgoing
+                , expectCliFieldEqual status Pending
                 ]
+            let txId = getTxId txJson
 
-        -- forget transaction
-        fromExit <$> deleteTransactionViaCLI @t ctx wSrcId txId
-            `shouldReturn` ExitSuccess
-
-        -- verify again balance on src wallet
-        (fromStdout <$> getWalletViaCLI @t ctx wSrcId)
-            >>= expectValidJSON (Proxy @ApiWallet)
-            >>= flip verify
-                [ expectCliFieldEqual balanceAvailable faucetAmt
-                ]
+            -- forget transaction
+            fromExit <$> deleteTransactionViaCLI @t ctx wSrcId txId
+                `shouldReturn` ExitSuccess
+
+            -- verify again balance on src wallet
+            (fromStdout <$> getWalletViaCLI @t ctx wSrcId)
+                >>= expectValidJSON (Proxy @ApiWallet)
+                >>= flip verify
+                    [ expectCliFieldEqual balanceAvailable faucetAmt
+                    ]
 
         eventually_ $ do
             (fromStdout <$> listTransactionsViaCLI @t ctx [wSrcId])

lib/core/src/Cardano/Wallet/DaedalusIPC.hs

@@ -221,7 +221,6 @@ windowsReadMessage handle = do
     _int1 <- readInt32 handle
     _int2 <- readInt32 handle
     size <- readInt64 handle
-    -- logInfo $ "int is: " <> (show [_int1, _int2]) <> " and blob is: " <> (show blob)
     BL.hGet handle $ fromIntegral size
   where
     readInt64 :: Handle -> IO Word64

lib/jormungandr/src/Cardano/Wallet/Jormungandr.hs

@@ -91,7 +91,7 @@ import Cardano.Wallet.Primitive.Types
 import Cardano.Wallet.Transaction
     ( TransactionLayer )
 import Control.Concurrent
-    ( forkFinally )
+    ( forkFinally, threadDelay )
 import Control.Concurrent.Async
     ( race )
 import Control.DeepSeq
@@ -160,7 +160,14 @@ serveWallet (cfg, tr) sTolerance databaseDir hostPref listen lj beforeMainLoop =
         Left e -> handleApiServerStartupError e
         Right (wPort, socket) -> either (const ExitSuccess) id <$> do
             let tracerIPC = appendName "daedalus-ipc" tr
-            race (daedalusIPC tracerIPC wPort) $ do
+            -- FIXME
+            -- There's an ugly pause of 1s here for lack of a better fix. There
+            -- seem to be a race-condition on Daedalus' side which isn't able to
+            -- pick up the 'Started' message if we send it too quickly...
+            -- There shouldn't be any issue for us to resend the 'Started'
+            -- message multiple time if we haven't received 'QueryPort' after a
+            -- while and that would be a better way to fix this!
+            race (threadDelay oneSecond *> daedalusIPC tracerIPC wPort) $ do
                 withNetworkLayer tr lj $ \case
                     Left e -> handleNetworkStartupError e
                     Right (cp, nl) -> do
@@ -313,6 +320,10 @@ serveWallet (cfg, tr) sTolerance databaseDir hostPref listen lj beforeMainLoop =
 -- Exported Utilities
 --------------------------------------------------------------------------------
 
+-- | One second in micro seconds
+oneSecond :: Int
+oneSecond = 1000 * 1000
+
 -- | Covert a raw block to one that the "Cardano.Pool.Metrics" module accepts.
 toSPBlock :: J.Block -> Pool.Block
 toSPBlock b = Pool.Block

lib/jormungandr/src/Cardano/Wallet/Jormungandr/Api/Client.hs

@@ -78,7 +78,7 @@ import Cardano.Wallet.Primitive.Types
 import Control.Arrow
     ( left )
 import Control.Exception
-    ( Exception )
+    ( Exception, SomeException, handle, throwIO )
 import Control.Monad
     ( void )
 import Control.Monad.Catch
@@ -87,6 +87,8 @@ import Control.Monad.Trans.Except
     ( ExceptT (..), throwE, withExceptT )
 import Data.Coerce
     ( coerce )
+import Data.List
+    ( isSubsequenceOf )
 import Data.Maybe
     ( mapMaybe )
 import Data.Proxy
@@ -252,7 +254,15 @@ mkJormungandrClient mgr baseUrl = JormungandrClient
     }
   where
     run :: ClientM a -> IO (Either ServantError a)
-    run query = runClientM query (mkClientEnv mgr baseUrl)
+    run query = handle windowsNetworkException $
+        runClientM query (mkClientEnv mgr baseUrl)
+      where
+        windowsNetworkException :: SomeException -> IO (Either ServantError a)
+        windowsNetworkException e
+            | "WSAECONNREFUSED" `isSubsequenceOf` show e =
+                pure $ Left $ ConnectionError $ T.pack $ show e
+            | otherwise =
+                throwIO e
 
     defaultHandler
         :: Link

lib/jormungandr/test/data/jormungandr/genesis.yaml

@@ -24,7 +24,7 @@ blockchain_configuration:
 
   # The slot duration, in seconds, is the time between the creation
   # of 2 blocks
-  slot_duration: 1
+  slot_duration: 2
 
   # The number of blocks (*10) per epoch
   epoch_stability_depth: 4