Repository: aurora
Updated Branches:
refs/heads/master b25ab8745 -> 6b768bd3d
Fix flaky `ServerSetImplTest` test.
The `testUnwatchOnException` test method uses a forced
InterruptedException to test that watches are un-registered. In doing
so, the test inadvertantly set the interrupt bit for the test runner
thread, poisoning subsequent tests that invoked blocking code. The
poisoning was only evident when the test methods were not run in lexical
order, which is the case in the vagrant vm. This fix explicitly clears
the interrupt bit for the test thread with an explanation of why this is
done.
Testing Done:
Before the fix, this consistent error in the vagrant VM:
```
vagrant@aurora:~/aurora$ ./gradlew --rerun-tasks commons:test --tests
org.apache.aurora.common.zookeeper.ServerSetImplTest
...
org.apache.aurora.common.zookeeper.ServerSetImplTest > testOrdering FAILED
org.apache.aurora.common.net.pool.DynamicHostSet$MonitorException at
ServerSetImplTest.java:155
Caused by: org.apache.aurora.common.zookeeper.Group$WatchException at
ServerSetImplTest.java:155
Caused by: org.apache.aurora.common.zookeeper.Group$JoinException
at ServerSetImplTest.java:155
Caused by: java.lang.InterruptedException at
ServerSetImplTest.java:155
```
Green after the fix in the vm and when run normally on my machine.
Bugs closed: AURORA-1574
Reviewed at https://reviews.apache.org/r/42102/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/6b768bd3
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/6b768bd3
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/6b768bd3
Branch: refs/heads/master
Commit: 6b768bd3dd053b776a5b713c4fd5ce95e31e666b
Parents: b25ab87
Author: John Sirois <[email protected]>
Authored: Sat Jan 9 11:10:38 2016 -0700
Committer: John Sirois <[email protected]>
Committed: Sat Jan 9 11:10:38 2016 -0700
----------------------------------------------------------------------
.../common/zookeeper/ServerSetImplTest.java | 22 ++++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/6b768bd3/commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java
----------------------------------------------------------------------
diff --git
a/commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java
b/commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java
index d446a99..56cc32d 100644
---
a/commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java
+++
b/commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java
@@ -258,7 +258,7 @@ public class ServerSetImplTest extends BaseZooKeeperTest {
expect(zkClient.registerExpirationHandler(anyObject(Command.class)))
.andReturn(onExpirationWatcher);
- expect(zkClient.get()).andThrow(new InterruptedException());
+ expect(zkClient.get()).andThrow(new InterruptedException()); // See
interrupted() note below.
expect(zkClient.unregister(onExpirationWatcher)).andReturn(true);
control.replay();
@@ -269,7 +269,14 @@ public class ServerSetImplTest extends BaseZooKeeperTest {
serverset.watch(hostSet -> {});
fail("Expected MonitorException");
} catch (DynamicHostSet.MonitorException e) {
- // expected
+ // NB: The assert is not important to this test, but the call to
`Thread.interrupted()` is.
+ // That call both returns the current interrupted status as well as
clearing it. The clearing
+ // is crucial depending on the order tests are run in this class. If
this test runs before
+ // one of the tests above that uses a `ZooKeeperClient` for example,
those tests will fail
+ // executing `ZooKeeperClient.get` which internally blocks on s
sync-point that takes part in
+ // the interruption mechanism and so immediately throws
`InterruptedException` based on the
+ // un-cleared interrupted bit.
+ assertTrue(Thread.interrupted());
}
control.verify();
}
@@ -281,15 +288,8 @@ public class ServerSetImplTest extends BaseZooKeeperTest {
private ServerSet.EndpointStatus join(ServerSet serverSet, String host)
throws JoinException, InterruptedException {
- return serverSet.join(InetSocketAddress.createUnresolved(host, 42),
ImmutableMap.<String, InetSocketAddress>of());
- }
-
- private void assertChangeFired(Map<InetSocketAddress, Status> hostsStatuses)
- throws InterruptedException {
- assertChangeFired(
-
ImmutableSet.copyOf(Iterables.transform(ImmutableSet.copyOf(hostsStatuses.entrySet()),
- e -> new ServiceInstance(new Endpoint(e.getKey().getHostName(),
e.getKey().getPort()),
- ImmutableMap.<String, Endpoint>of(), e.getValue()))));
+ return serverSet.join(
+ InetSocketAddress.createUnresolved(host, 42), ImmutableMap.<String,
InetSocketAddress>of());
}
private void assertChangeFired(String... serviceHosts)
