Skip to content

Implement tests for registy monitoring (DP-100) #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 110 additions & 2 deletions src/Control/Distributed/Process/Tests/CH.hs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Control.Concurrent.MVar
, takeMVar
, readMVar
)
import Control.Monad (replicateM_, replicateM, forever, void, unless)
import Control.Monad (replicateM_, replicateM, forever, void, unless, when)
import Control.Exception (SomeException, throwIO)
import qualified Control.Exception as Ex (catch)
import Control.Applicative ((<$>), (<*>), pure, (<|>))
Expand All @@ -31,6 +31,8 @@ import Control.Distributed.Process.Internal.Types
)
import Control.Distributed.Process.Node
import Control.Distributed.Process.Serializable (Serializable)
import Control.Distributed.Process.Debug
import Control.Distributed.Process.Management.Internal.Types

import Test.HUnit (Assertion, assertFailure)
import Test.Framework (Test, testGroup)
Expand Down Expand Up @@ -1316,6 +1318,111 @@ testUnsafeSendChan TestTransport{..} = do

takeMVar clientDone

testRegistryMonitoring :: TestTransport -> Assertion
testRegistryMonitoring TestTransport{..} = do
localNode <- newLocalNode testTransport initRemoteTable
remoteNode <- newLocalNode testTransport initRemoteTable
return ()

-- Local process. Test if local process will be removed from
-- registry when it dies.
box <- newEmptyMVar
runProcess localNode $ do
pid <- spawnLocal $ do
expect
register "test" pid
tpid <- whereis "test"
if tpid == Just pid
then do _ <- monitor pid
send pid ()
ProcessMonitorNotification{} <- expect
tpid1 <- whereis "test"
liftIO $ putMVar box (Nothing == tpid1)
else liftIO $ putMVar box False
True <- takeMVar box
return ()

-- Remote process. Test if remote process entry is removed
-- from registry when process dies.
remote1 <- testRemote remoteNode
runProcess localNode $
let waitpoll = do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not busy wait, even in tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I know how to write this test then. Because linking or monitoring will interfere with test, I.e. if I'll link to process then process is guaranteed to be removed from registry, and I want to test if it will be removed even if no one is linked to it.
I'll need to think about this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with monitoring, as you did in the local case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for sanity I need to remove monitor in local case also. (Monitor there just not affect the algorithm, but I'm testing semantics here, so I should pretend that d-p in black box).

According to our (and erlang) specification if ProcessDead notification arrives to node, than all instances of Process should be removed from internal structures (registry...). If I'm explicitly linked to remote process and that process is dead than it's guaranteed that I'll eventually get ProcessDead notification. In this test I want to show that remote process will be removed from the registry even if it's not explicitly linked/monitor it. So explicit monitor breaks the test.

What I've think that having such semantics we may want to monitor registry values somehow, and wait for such event. I think I'll need to check code once again, it's quite possible that we already have this option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the property you want to check is that the process gets removed from the registry, whether you monitor it or not. The test for the local test looks good to me. Monitors are independent of each other. So setting a monitor for the purposes of the test shouldn't interfere or otherwise paper over any missing monitors inside the code under test if any.

w <- whereis "test" :: Process (Maybe ProcessId)
forM_ w (const waitpoll)
in do register "test" remote1
send remote1 ()
waitpoll
return ()
return ()

-- Many labels. Test if all labels associated with process
-- are removed from registry when it dies.
remote2 <- testRemote remoteNode
runProcess localNode $
let waitpoll = do
w1 <- whereis "test-3" :: Process (Maybe ProcessId)
w2 <- whereis "test-4" :: Process (Maybe ProcessId)
forM_ (w1 <|> w2) (const waitpoll)
in do register "test-3" remote2
register "test-4" remote2
send remote2 ()
waitpoll
return ()

{- XXX: waiting including patch for nsend for remote process
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can uncomment now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it will go away during rebase.

remote3 <- testRemote remoteNode
remote4 <- testRemote remoteNode
-- test many labels
runProcess localNode $ do
register "test-3" remote3
reregister "test-3" remote4
send remote3 ()
liftIO $ threadDelay 50000 -- XXX: racy
monitor remote4
nsend "test-3" ()
ProcessMonitorNotification{} <- expect
return ()
-}

-- Test registerRemoteAsync properties. Add a local process to
-- remote registry and checks that it is removed
-- when the process dies.
remote5 <- testRemote remoteNode
runProcess localNode $ do
registerRemoteAsync (localNodeId remoteNode) "test" remote5
RegisterReply _ True <- expect
send remote5 ()
let waitpoll = do
whereisRemoteAsync (localNodeId remoteNode) "test"
WhereIsReply _ mr <- expect
forM_ mr (const waitpoll)
waitpoll

-- Add remote process to remote registry and checks if
-- entry is removed then process is dead.
remote6 <- testRemote localNode
runProcess localNode $ do
registerRemoteAsync (localNodeId remoteNode) "test" remote6
RegisterReply _ True <- expect
send remote6 ()
let waitpoll = do
whereisRemoteAsync (localNodeId remoteNode) "test"
WhereIsReply _ mr <- expect
forM_ mr (const waitpoll)
waitpoll
where
testRemote node = do
-- test many labels
pidBox <- newEmptyMVar
forkProcess node $ do
us <- getSelfPid
liftIO $ putMVar pidBox us
expect :: Process ()
takeMVar pidBox

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extraneous whitespace.




tests :: TestTransport -> IO [Test]
tests testtrans = return [
testGroup "Basic features" [
Expand Down Expand Up @@ -1347,12 +1454,13 @@ tests testtrans = return [
, testCase "MaskRestoreScope" (testMaskRestoreScope testtrans)
, testCase "ExitLocal" (testExitLocal testtrans)
, testCase "ExitRemote" (testExitRemote testtrans)
, testCase "TestRegistryMonitor" (testRegistryMonitoring testtrans)
-- Unsafe Primitives
, testCase "TestUnsafeSend" (testUnsafeSend testtrans)
, testCase "TestUnsafeNSend" (testUnsafeNSend testtrans)
, testCase "TestUnsafeSendChan" (testUnsafeSendChan testtrans)
-- usend
, testCase "USend" (testUSend testtrans 50)
-- , testCase "USend" (testUSend testtrans 50)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to XXX comment above, so should go away as usend is merged

]
, testGroup "Monitoring and Linking" [
-- Monitoring processes
Expand Down