-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, (<|>)) | ||
|
@@ -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) | ||
|
@@ -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 | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can uncomment now? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extraneous whitespace. |
||
|
||
|
||
|
||
tests :: TestTransport -> IO [Test] | ||
tests testtrans = return [ | ||
testGroup "Basic features" [ | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change intended? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Related to XXX comment above, so should go away as |
||
] | ||
, testGroup "Monitoring and Linking" [ | ||
-- Monitoring processes | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ofProcess
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 getProcessDead
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.
There was a problem hiding this comment.
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.