From efba97581359eba485d074b4b7a715d7851f1f82 Mon Sep 17 00:00:00 2001
From: Chris Beams <chris@beams.io>
Date: Fri, 10 Jan 2020 23:31:27 +0100
Subject: [PATCH] Avoid race condition in LocalBitcoinNodeTests

Previously, Travis CI was failing non-deterministically due to a race
condition in which a thread was started in order to call the blocking
ServerSocket.accept() method, and sometimes the subsequent attempt by
LocalBitcoinNode.detectAndRun() to connect to that socket's port would
occur before the thread had actually called the accept() method.

This commit simplifies the approach by removing the thread entirely. As
it turns out, calling accept() is not necessary; simply constructing a
new ServerSocket() binds to and listens on the given port, such that a
subsequent attempt to connect() will succeed.
---
 .../core/btc/nodes/LocalBitcoinNodeTests.java | 26 ++++++-------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/core/src/test/java/bisq/core/btc/nodes/LocalBitcoinNodeTests.java b/core/src/test/java/bisq/core/btc/nodes/LocalBitcoinNodeTests.java
index 32ddaaee54..9233263fd7 100644
--- a/core/src/test/java/bisq/core/btc/nodes/LocalBitcoinNodeTests.java
+++ b/core/src/test/java/bisq/core/btc/nodes/LocalBitcoinNodeTests.java
@@ -3,7 +3,6 @@ package bisq.core.btc.nodes;
 import java.net.ServerSocket;
 
 import java.io.IOException;
-import java.io.UncheckedIOException;
 
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -17,37 +16,28 @@ public class LocalBitcoinNodeTests {
 
     private AtomicBoolean called = new AtomicBoolean(false);
     private Runnable callback = () -> called.set(true);
-    private int port;
+    private ServerSocket socket;
     private LocalBitcoinNode localBitcoinNode;
 
     @Before
     public void setUp() throws IOException {
-        // Find an available port
-        try (ServerSocket socket = new ServerSocket(0)) {
-            port = socket.getLocalPort();
-        }
-        localBitcoinNode = new LocalBitcoinNode(port);
+        // Bind to and listen on an available port
+        socket = new ServerSocket(0);
+        localBitcoinNode = new LocalBitcoinNode(socket.getLocalPort());
     }
 
     @Test
     public void whenLocalBitcoinNodeIsDetected_thenCallbackGetsRun_andIsDetectedReturnsTrue() {
-        // Listen on the port, indicating that the local bitcoin node is running
-        new Thread(() -> {
-            try (ServerSocket socket = new ServerSocket(port)){
-                socket.accept();
-            } catch (IOException ex) {
-                throw new UncheckedIOException(ex);
-            }
-        }).start();
-
+        // Continue listening on the port, indicating the local Bitcoin node is running
         localBitcoinNode.detectAndRun(callback);
         assertTrue(called.get());
         assertTrue(localBitcoinNode.isDetected());
     }
 
     @Test
-    public void whenLocalBitcoinNodeIsNotDetected_thenCallbackGetsRun_andIsDetectedReturnsFalse() {
-        // Leave port unbound, indicating that no local Bitcoin node is running
+    public void whenLocalBitcoinNodeIsNotDetected_thenCallbackGetsRun_andIsDetectedReturnsFalse() throws IOException {
+        // Stop listening on the port, indicating the local Bitcoin node is not running
+        socket.close();
         localBitcoinNode.detectAndRun(callback);
         assertTrue(called.get());
         assertFalse(localBitcoinNode.isDetected());