Make sure to subscribe before starting subscriber thread in tests

5 files changed, 25 insertions, 23 deletions

diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/DelayedResponseHandler.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/DelayedResponseHandler.java
index 21909a5e371..4c8fa1afe0d 100644
--- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/DelayedResponseHandler.java
+++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/DelayedResponseHandler.java
@@ -12,7 +12,7 @@ import java.util.logging.Logger;
 
 /**
  * The run method of this class is executed periodically to return delayed responses
- * (long polling requests that are about to time out and needs to be returned).
+ * (requests use long polling, so config proxy needs to return a response when they time out).
  *
  * @author hmusum
  */
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java
index 4b92fbbe81a..1502e01a4b0 100644
--- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java
+++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java
@@ -116,7 +116,7 @@ public class ProxyServer implements Runnable {
         }
     }
 
-    public RawConfig resolveConfig(JRTServerConfigRequest req) {
+    RawConfig resolveConfig(JRTServerConfigRequest req) {
         statistics.incProcessedRequests();
         // Calling getConfig() will either return with an answer immediately or
         // create a background thread that retrieves config from the server and
@@ -247,12 +247,12 @@ public class ProxyServer implements Runnable {
 
     // Cancels all config instances and flushes the cache. When this method returns,
     // the cache will not be updated again before someone calls getConfig().
-    synchronized void flush() {
+    private synchronized void flush() {
         memoryCache.clear();
         configClient.cancel();
     }
 
-    public void stop() {
+    void stop() {
         Event.stopping("configproxy", "shutdown");
         if (rpcServer != null) rpcServer.shutdown();
         if (delayedResponseScheduler != null) delayedResponseScheduler.cancel(true);
@@ -274,7 +274,7 @@ public class ProxyServer implements Runnable {
         return configClient.getSourceConnections();
     }
 
-    public void updateSourceConnections(List<String> sources) {
+    void updateSourceConnections(List<String> sources) {
         configSource = new ConfigSourceSet(sources);
         flush();
         configClient = createRpcClient();
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java
index 83e11e343be..4863fd24d4e 100644
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java
@@ -51,7 +51,7 @@ public class MapBackedConfigSource implements ConfigSource, ConfigSourceClient {
     public void shutdownSourceConnections() {
     }
 
-    public void clear() {
+    void clear() {
         backing.clear();
     }
 
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java
index c21fad062eb..276c7c07cb7 100644
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java
@@ -21,7 +21,7 @@ import static org.junit.Assert.*;
  */
 public class ProxyServerTest {
 
-    MemoryCache memoryCache = new MemoryCache();
+    private final MemoryCache memoryCache = new MemoryCache();
     private final MapBackedConfigSource source = new MapBackedConfigSource(UpstreamConfigSubscriberTest.MockClientUpdater.create(memoryCache));
     private ProxyServer proxy = ProxyServer.createTestServer(source, source, memoryCache);
 
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriberTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriberTest.java
index e566df8bb2a..92ace6c58d1 100644
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriberTest.java
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriberTest.java
@@ -34,7 +34,6 @@ public class UpstreamConfigSubscriberTest {
     private static RawConfig errorConfig;
     private static ConfigKey<?> errorConfigKey;
     private static Payload fooPayload;
-    private static Payload errorPayload;
     private long generation = 1;
 
 
@@ -51,7 +50,7 @@ public class UpstreamConfigSubscriberTest {
         fooConfig = new RawConfig(Helper.fooConfig.getKey(), Helper.fooConfig.getDefMd5(), fooPayload, ConfigUtils.getMd5(payload), generation, 0, Helper.fooConfig.getDefContent(), Optional.empty());
 
         payload = new ConfigPayload(new Slime());
-        errorPayload = Payload.from(payload);
+        Payload errorPayload = Payload.from(payload);
         errorConfigKey = new ConfigKey<>("error", fooConfig.getConfigId(), fooConfig.getNamespace());
         errorConfig = new RawConfig(errorConfigKey, fooConfig.getDefMd5(), errorPayload, ConfigUtils.getMd5(payload), generation, ErrorCode.UNKNOWN_DEFINITION, fooConfig.getDefContent(), Optional.empty());
 
@@ -61,16 +60,9 @@ public class UpstreamConfigSubscriberTest {
         mockConnection = new MockConnection(sourceResponses);
     }
 
-    private ConfigPayload getConfigPayload(String key, String value) {
-        Slime slime = new Slime();
-        slime.setObject().setString(key, value);
-        return new ConfigPayload(slime);
-    }
-
     @Test
     public void basic() {
-        final UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(fooConfig);
-        new Thread(subscriber).start();
+        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber();
         waitForConfigGeneration(clientUpdater, generation);
         assertThat(clientUpdater.getLastConfig(), is(fooConfig));
         subscriber.cancel();
@@ -78,9 +70,7 @@ public class UpstreamConfigSubscriberTest {
 
     @Test
     public void require_that_reconfiguration_works() {
-        final UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(fooConfig);
-
-        new Thread(subscriber).start();
+        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber();
         waitForConfigGeneration(clientUpdater, generation);
         assertThat(clientUpdater.getLastConfig(), is(fooConfig));
 
@@ -99,9 +89,7 @@ public class UpstreamConfigSubscriberTest {
     @Test
     public void require_that_error_response_is_handled() {
         sourceResponses.put(errorConfigKey, errorConfig);
-        final UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(fooConfig);
-
-        new Thread(subscriber).start();
+        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber();
         waitForConfigGeneration(clientUpdater, generation);
         RawConfig lastConfig = clientUpdater.getLastConfig();
         assertThat(lastConfig, is(errorConfig));
@@ -166,4 +154,18 @@ public class UpstreamConfigSubscriberTest {
             return lastConfig;
         }
     }
+
+    private UpstreamConfigSubscriber createUpstreamConfigSubscriber() {
+        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(fooConfig);
+        subscriber.subscribe();
+        new Thread(subscriber).start();
+        return subscriber;
+    }
+
+    private ConfigPayload getConfigPayload(String key, String value) {
+        Slime slime = new Slime();
+        slime.setObject().setString(key, value);
+        return new ConfigPayload(slime);
+    }
+
 }