Run tests using network-transport-inmemory #19

qnikst · 2015-07-03T11:50:58Z

This patch provides an ability to test distributed-process using inmemory backend, this could reduce a change of network failures because of problem in n-t implementation.

facundominguez · 2015-07-03T11:56:21Z

According to semantics network failures is guaranteed to be
noticed by network-transport iff communication happened after
connection failure.

Is this documented? I don't remember anything of the sort.

qnikst · 2015-07-03T11:57:44Z

We don't have any documentation for n-t semantics, this may be a good time to create that.
But this assumption us the only sane one.

facundominguez · 2015-07-03T13:04:42Z

Shouldn't we have tests built and run with both inmemory and TCP for now?

qnikst · 2015-07-03T13:06:15Z

I think the idea was to make CI reliable, with tcp implementation 1 of 4 version fails, because of internal races (or possibly not well written test). I thinks I can leave 2 options, but disable tcp by default until it will not work reliably. Does that make sense?

facundominguez · 2015-07-03T13:11:56Z

In that case, perhaps it is better to have the tests fail until the problems are fixed.
Wouldn't disabling tcp tests give a false sense of stability?

mboes · 2015-07-03T13:19:11Z

I think we should use inmemory by default, and have CI test both inmemory and tcp.

mboes · 2015-07-03T13:30:01Z

There is no semantics to refer to in this case. So we shouldn't be citing any.

mboes · 2015-07-03T13:33:36Z

src/Control/Distributed/Process/Tests/CH.hs

@@ -734,7 +745,8 @@ testReconnect TestTransport{..} = do


    -- Simulate network failure
-    liftIO $ testBreakConnection (nodeAddress nid1) (nodeAddress nid2)
+    liftIO $ do testBreakConnection (nodeAddress nid1) (nodeAddress nid2)
+                threadDelay 1000000


threadDelay is so evil. Because it doesn't guarantee the absence of races. And slows tests down. Can we avoid this?

qnikst · 2015-07-03T15:15:14Z

This patch depend on haskell-distributed/distributed-process#246

facundominguez · 2015-07-03T19:50:59Z

Only TestCHInTCP uses TCP? What about TestStats, TestMx, etc?

qnikst · 2015-07-03T19:52:21Z

Those tests do not depend on network-backend implementation, I see to reason in testing them on multiple backends. Correct me if I'm wrong.

facundominguez · 2015-07-03T20:06:58Z

Alright. I'm not familiar with those. I gave them a look and testing with inmemory looks ok for now. This could change, say, if we ever find that the things they test don't work with tcp.

mboes · 2015-07-03T21:07:42Z

distributed-process-tests.cabal

@@ -11,6 +11,10 @@ category:      Control, Cloud Haskell
 build-type:    Simple
 cabal-version: >=1.8

+flag tcp-test


Or simply flag tcp, since this is a test package.

qnikst · 2015-07-06T08:34:13Z

Addressed all comments, could merge?

qnikst · 2015-07-06T09:04:29Z

@mboes, your comment lost during rewrite:

threadDelay is so evil. Because it doesn't guarantee the absence of races. And slows tests down. Can we avoid this?

Yes we could there are 2 approaches here:

use node monitoring, and run registerRemoteAsync after we have seen
that connection failed.
use loop in registerRemoteAsync operation

ref <- registerRemoteAsync 
let loop = do mx <-  receiveTimeout n  [...]
                     maybe loop (return x) mx
loop

should I do that? This is needed because handling connection failure on d-p layer is an asynchronous operation.

facundominguez · 2015-07-06T11:42:37Z

src/Control/Distributed/Process/Tests/CH.hs

@@ -302,19 +302,25 @@ testMonitorRemoteDeadProcess TestTransport{..} mOrL un = do
 testMonitorDisconnect :: TestTransport -> Bool -> Bool -> Assertion
 testMonitorDisconnect TestTransport{..} mOrL un = do
  processAddr <- newEmptyMVar
+  processAddr2 <- newEmptyMVar
  monitorSetup <- newEmptyMVar
  done <- newEmptyMVar

  forkIO $ do
    localNode <- newLocalNode testTransport initRemoteTable
    addr <- forkProcess localNode . liftIO $ threadDelay 1000000


I don't understand this threadDelay. Why not just block forever with expect?

looks reasonable (it was not mine code)..

Network-transport are not guaranteed to emit connection failure event unless there were communication between local and remote endpoints. As a result additional communication was introduced in order introduce that communication.

qnikst · 2015-07-06T17:17:54Z

Addressed all comments.

facundominguez · 2015-07-06T17:21:26Z

👍

Run tests using network-transport-inmemory

qnikst assigned facundominguez Jul 3, 2015

mboes reviewed Jul 3, 2015
View reviewed changes

qnikst force-pushed the inmemory-tests branch from 9ecdd20 to aac4311 Compare July 3, 2015 15:05

qnikst changed the title ~~Inmemory tests~~ Implement tests based on n-t-inmemory backend Jul 3, 2015

qnikst mentioned this pull request Jul 3, 2015

Use inmemory and tcp backend in CI haskell-distributed/distributed-process#247

Closed

mboes changed the title ~~Implement tests based on n-t-inmemory backend~~ Run tests using network-transport-inmemory Jul 3, 2015

qnikst force-pushed the inmemory-tests branch from aac4311 to e8fc039 Compare July 3, 2015 16:53

mboes reviewed Jul 3, 2015
View reviewed changes

Use in-memory backend for the distributed process tests.

da3cf40

qnikst force-pushed the inmemory-tests branch from e8fc039 to 7719bc0 Compare July 6, 2015 08:33

facundominguez reviewed Jul 6, 2015
View reviewed changes

Add additional communication in monitor tests.

14782a9

Network-transport are not guaranteed to emit connection failure event unless there were communication between local and remote endpoints. As a result additional communication was introduced in order introduce that communication.

qnikst force-pushed the inmemory-tests branch from 7719bc0 to 14782a9 Compare July 6, 2015 17:17

qnikst added a commit that referenced this pull request Jul 6, 2015

Merge pull request #19 from haskell-distributed/inmemory-tests

f403439

Run tests using network-transport-inmemory

qnikst merged commit f403439 into master Jul 6, 2015

qnikst deleted the inmemory-tests branch July 6, 2015 17:30

Run tests using network-transport-inmemory #19

Run tests using network-transport-inmemory #19

Uh oh!

Conversation

qnikst commented Jul 3, 2015

Uh oh!

facundominguez commented Jul 3, 2015

Uh oh!

qnikst commented Jul 3, 2015

Uh oh!

facundominguez commented Jul 3, 2015

Uh oh!

qnikst commented Jul 3, 2015

Uh oh!

facundominguez commented Jul 3, 2015

Uh oh!

mboes commented Jul 3, 2015

Uh oh!

mboes commented Jul 3, 2015

Uh oh!

mboes Jul 3, 2015

Choose a reason for hiding this comment

Uh oh!

qnikst commented Jul 3, 2015

Uh oh!

facundominguez commented Jul 3, 2015

Uh oh!

qnikst commented Jul 3, 2015

Uh oh!

facundominguez commented Jul 3, 2015

Uh oh!

mboes Jul 3, 2015

Choose a reason for hiding this comment

Uh oh!

qnikst commented Jul 6, 2015

Uh oh!

qnikst commented Jul 6, 2015

Uh oh!

facundominguez Jul 6, 2015

Choose a reason for hiding this comment

Uh oh!

qnikst Jul 6, 2015

Choose a reason for hiding this comment

Uh oh!

qnikst commented Jul 6, 2015

Uh oh!

facundominguez commented Jul 6, 2015

Uh oh!

Uh oh!