From d9e4acf5d083c32b0beea6c34b98bf1fd1b7a284 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 16 Mar 2017 21:56:43 +0100 Subject: 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 --- .../config/proxy/DelayedResponseHandlerTest.java | 2 +- .../vespa/config/proxy/MockClientUpdater.java | 51 +++++++ .../yahoo/vespa/config/proxy/MockConnection.java | 86 ----------- .../yahoo/vespa/config/proxy/ProxyServerTest.java | 18 ++- .../config/proxy/UpstreamConfigSubscriberTest.java | 168 --------------------- 5 files changed, 69 insertions(+), 256 deletions(-) create mode 100644 config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockClientUpdater.java delete mode 100644 config-proxy/src/test/java/com/yahoo/vespa/config/proxy/MockConnection.java delete mode 100644 config-proxy/src/test/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriberTest.java (limited to 'config-proxy/src') 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; @@ -140,6 +140,22 @@ public class ProxyServerTest { assertEquals(1, cacheManager.size()); } + @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(); 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 createRequesterPool() { - JRTConfigRequester request = JRTConfigRequester.get(mockConnection, timingValues); - - Map 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()); - } - -} -- cgit v1.2.3