This is an automated email from the ASF dual-hosted git repository.
fangmin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new b8c8937 ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest
b8c8937 is described below
commit b8c8937029ca7e15116e91c6a6c436368ac0f9bb
Author: Ilya Maykov <ilyam@fb.com>
AuthorDate: Thu Dec 20 22:07:14 2018 +0800
ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest
Made some changes to handle the random ENTRY_CREATE events that occasionally fire even
though the watcher is created after the file is already written to disk. Should make the test
more stable.
Author: Ilya Maykov <ilyam@fb.com>
Closes #739 from ivmaykov/ZOOKEEPER-3219
---
.../zookeeper/common/FileChangeWatcherTest.java | 46 ++++++++++++++--------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
index e4950f3..2ef6a86 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/common/FileChangeWatcherTest.java
@@ -38,6 +38,7 @@ import org.slf4j.LoggerFactory;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
public class FileChangeWatcherTest extends ZKTestCase {
private static File tempDir;
@@ -70,6 +71,12 @@ public class FileChangeWatcherTest extends ZKTestCase {
tempDir.toPath(),
event -> {
LOG.info("Got an update: " + event.kind() + " " + event.context());
+ // Filter out the extra ENTRY_CREATE events that are
+ // sometimes seen at the start. Even though we create the watcher
+ // after the file exists, sometimes we still get a create event.
+ if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+ return;
+ }
synchronized (events) {
events.add(event);
events.notifyAll();
@@ -112,6 +119,12 @@ public class FileChangeWatcherTest extends ZKTestCase {
tempDir.toPath(),
event -> {
LOG.info("Got an update: " + event.kind() + " " + event.context());
+ // Filter out the extra ENTRY_CREATE events that are
+ // sometimes seen at the start. Even though we create the watcher
+ // after the file exists, sometimes we still get a create event.
+ if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+ return;
+ }
synchronized (events) {
events.add(event);
events.notifyAll();
@@ -184,6 +197,12 @@ public class FileChangeWatcherTest extends ZKTestCase {
tempDir.toPath(),
event -> {
LOG.info("Got an update: " + event.kind() + " " + event.context());
+ // Filter out the extra ENTRY_CREATE events that are
+ // sometimes seen at the start. Even though we create the watcher
+ // after the file exists, sometimes we still get a create event.
+ if (StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind())) {
+ return;
+ }
synchronized (events) {
events.add(event);
events.notifyAll();
@@ -214,21 +233,18 @@ public class FileChangeWatcherTest extends ZKTestCase {
public void testCallbackErrorDoesNotCrashWatcherThread() throws IOException, InterruptedException
{
FileChangeWatcher watcher = null;
try {
- final List<WatchEvent<?>> events = new ArrayList<>();
final AtomicInteger callCount = new AtomicInteger(0);
watcher = new FileChangeWatcher(
tempDir.toPath(),
event -> {
LOG.info("Got an update: " + event.kind() + " " + event.context());
+ int oldValue;
synchronized (callCount) {
- if (callCount.getAndIncrement() == 0) {
- callCount.notifyAll();
- throw new RuntimeException("This error should not crash the
watcher thread");
- }
+ oldValue = callCount.getAndIncrement();
+ callCount.notifyAll();
}
- synchronized (events) {
- events.add(event);
- events.notifyAll();
+ if (oldValue == 0) {
+ throw new RuntimeException("This error should not crash the watcher
thread");
}
});
watcher.start();
@@ -243,16 +259,14 @@ public class FileChangeWatcherTest extends ZKTestCase {
}
LOG.info("Modifying file again");
FileUtils.writeStringToFile(tempFile, "Hello world again\n", StandardCharsets.UTF_8,
true);
- synchronized (events) {
- if (events.isEmpty()) {
- events.wait(3000L);
+ synchronized (callCount) {
+ if (callCount.get() == 1) {
+ callCount.wait(3000L);
}
- assertEquals(2, callCount.get());
- assertFalse(events.isEmpty());
- WatchEvent<?> event = events.get(0);
- assertEquals(StandardWatchEventKinds.ENTRY_MODIFY, event.kind());
- assertEquals(tempFile.getName(), event.context().toString());
}
+ // The value of callCount can exceed 1 only if the callback thread
+ // survives the exception thrown by the first callback.
+ assertTrue(callCount.get() > 1);
} finally {
if (watcher != null) {
watcher.stop();
|