Github user cammckenzie commented on a diff in the pull request:
https://github.com/apache/curator/pull/262#discussion_r179001451
--- Diff:
curator-recipes/src/test/java/org/apache/curator/framework/recipes/leader/TestLeaderLatch.java
---
@@ -222,6 +223,35 @@ public void notLeader()
next.add(states.poll(timing.forSessionSleep().milliseconds(),
TimeUnit.MILLISECONDS));
next.add(states.poll(timing.forSessionSleep().milliseconds(),
TimeUnit.MILLISECONDS));
Assert.assertTrue(next.equals(Arrays.asList(ConnectionState.LOST.name(),
"false")) || next.equals(Arrays.asList("false", ConnectionState.LOST.name())),
next.toString());
+
+ latch.close();
+ client.close();
+
+ timing.sleepABit();
+ states.clear();
+ server = new TestingServer();
+ client = CuratorFrameworkFactory.builder()
+ .connectString(server.getConnectString())
+ .connectionTimeoutMs(1000)
+ .sessionTimeoutMs(timing.session())
+ .retryPolicy(new RetryOneTime(1))
+ .connectionStateErrorPolicy(new
SessionConnectionStateErrorPolicy())
+ .connectionHandlingPolicy(new
StandardConnectionHandlingPolicy(30))
+ .build();
+
client.getConnectionStateListenable().addListener(stateListener);
+ client.start();
+ latch = new LeaderLatch(client, "/test");
+ latch.addListener(listener);
+ latch.start();
+
Assert.assertEquals(states.poll(timing.forWaiting().milliseconds(),
TimeUnit.MILLISECONDS), ConnectionState.CONNECTED.name());
+
Assert.assertEquals(states.poll(timing.forWaiting().milliseconds(),
TimeUnit.MILLISECONDS), "true");
+ server.close();
+
Assert.assertEquals(states.poll(timing.forWaiting().milliseconds(),
TimeUnit.MILLISECONDS), ConnectionState.SUSPENDED.name());
+ next = Lists.newArrayList();
+
+ next.add(states.poll(timing.session() / 3,
TimeUnit.MILLISECONDS));
+ next.add(states.poll(timing.forSleepingABit().milliseconds(),
TimeUnit.MILLISECONDS));
+
Assert.assertTrue(next.equals(Arrays.asList(ConnectionState.LOST.name(),
"false")) || next.equals(Arrays.asList("false", ConnectionState.LOST.name())),
next.toString());
--- End diff --
The issue isn't really the LeaderLatch specifically is it? I think it's
probably cleaner to create a test case under curator-framework for the
ConnectionStateManager and then have a test explicitly for injected LOST
events, rather than have something in the LeaderLatch tests.
Because, this test doesn't really cover the use case that was discussed
originally (2 clients being leader at the same time).
---
