Remove some unnecessary test and move the useful one to another place

* Get rid of lots of hard to understand test code that didn't really
  work and try not to test implementation-specific stuff too

5 files changed, 69 insertions, 256 deletions

diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java
index 7ba4d676000..826a6d58639 100644
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/DelayedResponseHandlerTest.java
@@ -14,7 +14,7 @@ import static org.junit.Assert.assertThat;
  */
 public class DelayedResponseHandlerTest {
 
-    private final MapBackedConfigSource source = new MapBackedConfigSource(UpstreamConfigSubscriberTest.MockClientUpdater.create(new MemoryCache()));
+    private final MapBackedConfigSource source = new MapBackedConfigSource(new MockClientUpdater(new MemoryCache()));
 
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockClientUpdater.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockClientUpdater.java
new file mode 100644
index 00000000000..d8fb26c2e7e
--- /dev/null
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockClientUpdater.java
@@ -0,0 +1,51 @@
+package com.yahoo.vespa.config.proxy;
+
+import com.yahoo.vespa.config.ConfigKey;
+import com.yahoo.vespa.config.RawConfig;
+
+import java.time.Duration;
+import java.time.Instant;
+
+class MockClientUpdater extends ClientUpdater {
+    private RawConfig lastConfig;
+
+    MockClientUpdater(MemoryCache memoryCache) {
+        this(new ConfigProxyStatistics(), new Mode(), memoryCache);
+    }
+
+    private MockClientUpdater(ConfigProxyStatistics statistics, Mode mode, MemoryCache memoryCache) {
+        super(memoryCache, new MockRpcServer(), statistics, new DelayedResponses(statistics), mode);
+    }
+
+    @Override
+    public synchronized void updateSubscribers(RawConfig newConfig) {
+        lastConfig = newConfig;
+    }
+
+    synchronized RawConfig getLastConfig() {
+        return lastConfig;
+    }
+
+    long waitForConfigGeneration(ConfigKey<?> configKey, long expectedGeneration) {
+        Instant end = Instant.now().plus(Duration.ofSeconds(60));
+        RawConfig lastConfig;
+        do {
+            lastConfig = getLastConfig();
+            System.out.println("config=" + lastConfig + (lastConfig == null ? "" : ",generation=" + lastConfig.getGeneration()));
+            if (lastConfig != null && lastConfig.getKey().equals(configKey) &&
+                    lastConfig.getGeneration() == expectedGeneration) {
+                break;
+            } else {
+                try {
+                    Thread.sleep(10);
+                } catch (InterruptedException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        } while (Instant.now().isBefore(end));
+
+        if (lastConfig == null || ! lastConfig.getKey().equals(configKey) || lastConfig.getGeneration() != expectedGeneration)
+            throw new RuntimeException("Did not get config " + configKey + " with generation " + expectedGeneration);
+        return lastConfig.getGeneration();
+    }
+}
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConnection.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConnection.java
deleted file mode 100644
index c93a2e74258..00000000000
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConnection.java
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.config.proxy;
-
-import com.yahoo.jrt.Request;
-import com.yahoo.jrt.RequestWaiter;
-import com.yahoo.vespa.config.RawConfig;
-import com.yahoo.vespa.config.protocol.JRTServerConfigRequestV3;
-import com.yahoo.vespa.config.protocol.Payload;
-import com.yahoo.vespa.config.util.ConfigUtils;
-
-/**
- * For unit testing
- *
- * @author hmusum
- * @since 5.1.11
- */
-public class MockConnection extends com.yahoo.config.subscription.impl.MockConnection {
-
-    public MockConnection(MapBackedConfigSource configSource) {
-        this(new ProxyResponseHandler(configSource));
-    }
-
-    public MockConnection(ResponseHandler responseHandler) {
-        super(responseHandler);
-    }
-
-    static class ProxyResponseHandler implements ResponseHandler {
-        private RequestWaiter requestWaiter;
-        private Request request;
-        private final MapBackedConfigSource configSource;
-
-        protected ProxyResponseHandler(MapBackedConfigSource configSource) {
-            this.configSource = configSource;
-        }
-
-        @Override
-        public RequestWaiter requestWaiter() {
-            return requestWaiter;
-        }
-
-        @Override
-        public Request request() {
-            return request;
-        }
-
-        @Override
-        public ResponseHandler requestWaiter(RequestWaiter requestWaiter) {
-            this.requestWaiter = requestWaiter;
-            return this;
-        }
-
-        @Override
-        public ResponseHandler request(Request request) {
-            this.request = request;
-            return this;
-        }
-
-        @Override
-        public void run() {
-            if (request.isError()) {
-                System.out.println("Returning error response");
-                createErrorResponse();
-            } else {
-                System.out.println("Returning OK response");
-                createOkResponse();
-            }
-            requestWaiter.handleRequestDone(request);
-        }
-
-        protected void createOkResponse() {
-            JRTServerConfigRequestV3 jrtReq = JRTServerConfigRequestV3.createFromRequest(request);
-            long generation = 1;
-            RawConfig config = configSource.getConfig(jrtReq.getConfigKey());
-            if (config == null || config.getPayload() == null) {
-                throw new RuntimeException("No config for " + jrtReq.getConfigKey() + " found");
-            }
-            Payload payload = config.getPayload();
-            jrtReq.addOkResponse(payload, generation, ConfigUtils.getMd5(payload.getData()));
-        }
-
-        protected void createErrorResponse() {
-            JRTServerConfigRequestV3 jrtReq = JRTServerConfigRequestV3.createFromRequest(request);
-            jrtReq.addErrorResponse(request.errorCode(), request.errorMessage());
-        }
-    }
-}
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 58fcbaa0f65..f148e4951e1 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
@@ -22,7 +22,7 @@ import static org.junit.Assert.*;
 public class ProxyServerTest {
 
     private final MemoryCache memoryCache = new MemoryCache();
-    private final MapBackedConfigSource source = new MapBackedConfigSource(UpstreamConfigSubscriberTest.MockClientUpdater.create(memoryCache));
+    private final MapBackedConfigSource source = new MapBackedConfigSource(new MockClientUpdater(memoryCache));
     private ProxyServer proxy = ProxyServer.createTestServer(source, source, memoryCache);
 
     static final RawConfig fooConfig = Helper.fooConfig;
@@ -141,6 +141,22 @@ public class ProxyServerTest {
     }
 
     @Test
+    public void testReconfiguration() {
+        ConfigTester tester = new ConfigTester();
+        RawConfig res = proxy.resolveConfig(tester.createRequest(fooConfig));
+        assertNotNull(res);
+        assertThat(res.getPayload().toString(), is(Helper.fooPayload.toString()));
+
+        // Simulate deployment, add config with new config generation
+        long previousGeneration = res.getGeneration();
+        source.put(fooConfig.getKey(), createConfigWithNextConfigGeneration(res, 0));
+        JRTServerConfigRequest newRequestBasedOnResponse = tester.createRequest(res);
+        RawConfig res2 = proxy.resolveConfig(newRequestBasedOnResponse);
+        assertEquals(previousGeneration + 1, res2.getGeneration());
+        assertTrue(ProxyServer.configOrGenerationHasChanged(res2, newRequestBasedOnResponse));
+    }
+
+    @Test
     public void testReadingSystemProperties() {
         ProxyServer.Properties properties = ProxyServer.getSystemProperties();
         assertThat(properties.eventInterval, is(ConfigProxyStatistics.defaultEventInterval));
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
deleted file mode 100644
index 01733d0fd39..00000000000
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriberTest.java
+++ /dev/null
@@ -1,168 +0,0 @@
-// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.vespa.config.proxy;
-
-import com.yahoo.config.subscription.ConfigSourceSet;
-import com.yahoo.config.subscription.impl.JRTConfigRequester;
-import com.yahoo.slime.Slime;
-import com.yahoo.vespa.config.*;
-import com.yahoo.vespa.config.protocol.*;
-import com.yahoo.vespa.config.util.ConfigUtils;
-import org.junit.*;
-import org.junit.rules.TemporaryFolder;
-
-import java.time.Duration;
-import java.time.Instant;
-import java.util.LinkedHashMap;
-import java.util.Map;
-import java.util.Optional;
-
-import static junit.framework.TestCase.assertNotNull;
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-
-/**
- * @author hmusum
- */
-public class UpstreamConfigSubscriberTest {
-    private final ConfigSourceSet sourceSet = new ConfigSourceSet("tcp/foo:78");
-    private final TimingValues timingValues = ProxyServer.defaultTimingValues();
-
-    private MapBackedConfigSource sourceResponses;
-    private MockClientUpdater clientUpdater;
-    private MockConnection mockConnection;
-    private long generation = 1;
-
-
-    @Rule
-    public TemporaryFolder temporaryFolder = new TemporaryFolder();
-
-    @Before
-    public void setup() {
-        clientUpdater = MockClientUpdater.create(new MemoryCache());
-        sourceResponses = new MapBackedConfigSource(clientUpdater);
-        mockConnection = new MockConnection(sourceResponses);
-    }
-
-    @Test
-    public void basic() {
-        RawConfig fooConfig = Helper.fooConfig;
-        sourceResponses.put(fooConfig.getKey(), fooConfig);
-
-        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(fooConfig);
-        waitForConfigGeneration(clientUpdater, fooConfig.getKey(), generation);
-        assertThat(clientUpdater.getLastConfig(), is(fooConfig));
-        subscriber.cancel();
-    }
-
-    @Test
-    public void require_that_reconfiguration_works() {
-        RawConfig fooConfig = Helper.fooConfig;
-        sourceResponses.put(fooConfig.getKey(), fooConfig);
-
-        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(fooConfig);
-        waitForConfigGeneration(clientUpdater, fooConfig.getKey(), generation);
-        assertThat(clientUpdater.getLastConfig(), is(fooConfig));
-
-        // Update payload in config
-        generation++;
-        final ConfigPayload payload = Helper.createConfigPayload("bar", "value2");
-        RawConfig fooConfig2 = createRawConfig(fooConfig, payload);
-        sourceResponses.put(fooConfig2.getKey(), fooConfig2);
-
-        waitForConfigGeneration(clientUpdater, fooConfig2.getKey(), generation);
-        assertFalse(clientUpdater.getLastConfig().equals(fooConfig));
-        subscriber.cancel();
-    }
-
-    @Test
-    public void require_that_error_response_is_handled() {
-        // Create config with error based on fooConfig
-        RawConfig fooConfig = Helper.fooConfig;
-        ConfigPayload errorConfigPayload = new ConfigPayload(new Slime());
-        Payload errorPayload = Payload.from(errorConfigPayload);
-        RawConfig errorConfig = new RawConfig(fooConfig.getKey(), fooConfig.getDefMd5(), errorPayload,
-                                              ConfigUtils.getMd5(errorConfigPayload), generation,
-                                              ErrorCode.UNKNOWN_DEFINITION, fooConfig.getDefContent(), Optional.empty());
-        sourceResponses.put(fooConfig.getKey(), errorConfig);
-
-        UpstreamConfigSubscriber subscriber = createUpstreamConfigSubscriber(errorConfig);
-        waitForConfigGeneration(clientUpdater, fooConfig.getKey(), generation);
-        RawConfig lastConfig = clientUpdater.getLastConfig();
-        assertEquals(errorConfig, lastConfig);
-        assertThat(lastConfig.errorCode(), is(ErrorCode.UNKNOWN_DEFINITION));
-        subscriber.cancel();
-    }
-
-    private Map<ConfigSourceSet, JRTConfigRequester> createRequesterPool() {
-        JRTConfigRequester request = JRTConfigRequester.get(mockConnection, timingValues);
-
-        Map<ConfigSourceSet, JRTConfigRequester> requesterPool = new LinkedHashMap<>();
-        requesterPool.put(sourceSet, request);
-        return requesterPool;
-    }
-
-    private void waitForConfigGeneration(MockClientUpdater clientUpdater, ConfigKey<?> configKey, long expectedGeneration) {
-        Instant end = Instant.now().plus(Duration.ofSeconds(60));
-        RawConfig lastConfig;
-        do {
-            lastConfig = clientUpdater.getLastConfig();
-            System.out.println("config=" + lastConfig + (lastConfig == null ? "" : ",generation=" + lastConfig.getGeneration()));
-            if (lastConfig != null && lastConfig.getKey().equals(configKey) &&
-                    lastConfig.getGeneration() == expectedGeneration) {
-                break;
-            } else {
-                try {
-                    Thread.sleep(10);
-                } catch (InterruptedException e) {
-                    throw new RuntimeException(e);
-                }
-            }
-        } while (Instant.now().isBefore(end));
-
-        assertNotNull(lastConfig);
-        assertThat(lastConfig.getKey(), is(configKey));
-        assertThat(lastConfig.getGeneration(), is(expectedGeneration));
-    }
-
-    static class MockClientUpdater extends ClientUpdater {
-        private RawConfig lastConfig;
-
-        private MockClientUpdater(ConfigProxyStatistics statistics, Mode mode, MemoryCache memoryCache) {
-            super(memoryCache, new MockRpcServer(), statistics, new DelayedResponses(statistics), mode);
-        }
-
-        static MockClientUpdater create(MemoryCache memoryCache) {
-            Mode mode = new Mode();
-            ConfigProxyStatistics statistics = new ConfigProxyStatistics();
-            return new MockClientUpdater(statistics, mode, memoryCache);
-        }
-
-        @Override
-        public void updateSubscribers(RawConfig newConfig) {
-            lastConfig = newConfig;
-        }
-
-        RawConfig getLastConfig() {
-            return lastConfig;
-        }
-    }
-
-    private UpstreamConfigSubscriber createUpstreamConfigSubscriber(RawConfig config) {
-        UpstreamConfigSubscriber subscriber = new UpstreamConfigSubscriber(config, clientUpdater, sourceSet, timingValues, createRequesterPool());
-        subscriber.subscribe();
-        new Thread(subscriber).start();
-        return subscriber;
-    }
-
-    // Create new config based on another one
-    private RawConfig createRawConfig(RawConfig config, ConfigPayload configPayload) {
-        final int errorCode = 0;
-        Payload fooPayload = Payload.from(configPayload);
-        return new RawConfig(config.getKey(), config.getDefMd5(), fooPayload,
-                             ConfigUtils.getMd5(configPayload), generation, errorCode,
-                             config.getDefContent(), Optional.empty());
-    }
-
-}