From c537b48e6bd2d752f2892a9899dd1d36ae58e82b Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Mon, 26 Nov 2018 15:35:53 +0100 Subject: Remove Optional classes After review & new ideas: - All Optional classes have been removed: They must take a default argument to avoid having clients test whether the flag has been set or not, in order to allow local file flag source to undo a file flag setting from config server. - FeatureFlag is now backed by FlagSource::getString, and FlagSource::hasFeature has been removed. - FeatureFlag now allows default-true to facilitate default-enabled features --- .../com/yahoo/vespa/flags/FileFlagSourceTest.java | 53 +++++++++++++++++----- .../com/yahoo/vespa/flags/JacksonFlagTest.java | 6 --- 2 files changed, 41 insertions(+), 18 deletions(-) (limited to 'flags/src/test') diff --git a/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java b/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java index e422057f5fe..4b7c8c0bbb3 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/FileFlagSourceTest.java @@ -9,41 +9,70 @@ import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; +import static org.hamcrest.CoreMatchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; public class FileFlagSourceTest { private final FileSystem fileSystem = TestFileSystem.create(); private final FileFlagSource source = new FileFlagSource(fileSystem); + private final FlagId id = new FlagId("foo"); @Test - public void absentThenSet() throws IOException { - FlagId id = new FlagId("foo"); + public void testFeatureLikeFlags() throws IOException { FeatureFlag featureFlag = new FeatureFlag(id, source); + FeatureFlag byDefaultTrue = featureFlag.defaultToTrue(); + + assertFalse(featureFlag.value()); + assertTrue(byDefaultTrue.value()); + + writeFlag(id.toString(), "True\n"); + + assertTrue(featureFlag.value()); + assertTrue(byDefaultTrue.value()); + + writeFlag(id.toString(), "False\n"); + + assertFalse(featureFlag.value()); + assertFalse(byDefaultTrue.value()); + } + + @Test + public void testIntegerLikeFlags() throws IOException { StringFlag stringFlag = new StringFlag(id, "default", source); - OptionalStringFlag optionalStringFlag = new OptionalStringFlag(id, source); IntFlag intFlag = new IntFlag(id, -1, source); LongFlag longFlag = new LongFlag(id, -2L, source); - assertFalse(source.hasFeature(id)); assertFalse(source.getString(id).isPresent()); - assertFalse(featureFlag.isSet()); assertEquals("default", stringFlag.value()); - assertFalse(optionalStringFlag.value().isPresent()); assertEquals(-1, intFlag.value()); assertEquals(-2L, longFlag.value()); - Path featurePath = fileSystem.getPath(FileFlagSource.FLAGS_DIRECTORY).resolve(id.toString()); - Files.createDirectories(featurePath.getParent()); - Files.write(featurePath, "1\n".getBytes()); + writeFlag(id.toString(), "1\n"); - assertTrue(source.hasFeature(id)); assertTrue(source.getString(id).isPresent()); - assertTrue(featureFlag.isSet()); assertEquals("1\n", stringFlag.value()); - assertEquals("1\n", optionalStringFlag.value().get()); assertEquals(1, intFlag.value()); assertEquals(1L, longFlag.value()); } + + @Test + public void parseFailure() throws IOException { + FeatureFlag featureFlag = new FeatureFlag(id, source); + writeFlag(featureFlag.id().toString(), "garbage"); + + try { + featureFlag.value(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), containsString("garbage")); + } + } + + private void writeFlag(String flagId, String value) throws IOException { + Path featurePath = fileSystem.getPath(FileFlagSource.FLAGS_DIRECTORY).resolve(flagId); + Files.createDirectories(featurePath.getParent()); + Files.write(featurePath, value.getBytes()); + } } \ No newline at end of file diff --git a/flags/src/test/java/com/yahoo/vespa/flags/JacksonFlagTest.java b/flags/src/test/java/com/yahoo/vespa/flags/JacksonFlagTest.java index e4424d9886a..8bd486f5b47 100644 --- a/flags/src/test/java/com/yahoo/vespa/flags/JacksonFlagTest.java +++ b/flags/src/test/java/com/yahoo/vespa/flags/JacksonFlagTest.java @@ -9,8 +9,6 @@ import java.util.Objects; import java.util.Optional; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -19,7 +17,6 @@ public class JacksonFlagTest { private final ExampleJacksonClass defaultValue = new ExampleJacksonClass(); private final FlagSource source = mock(FlagSource.class); private final JacksonFlag jacksonFlag = new JacksonFlag<>(id.toString(), ExampleJacksonClass.class, defaultValue, source); - private final OptionalJacksonFlag optionalJacksonFlag = new OptionalJacksonFlag<>(id, ExampleJacksonClass.class, source); @Test public void unsetThenSet() { @@ -28,7 +25,6 @@ public class JacksonFlagTest { assertEquals(1, value.integer); assertEquals("2", value.string); assertEquals("3", value.dummy); - assertFalse(optionalJacksonFlag.value().isPresent()); when(source.getString(id)).thenReturn(Optional.of("{\"integer\": 4, \"string\": \"foo\", \"stray\": 6}")); value = jacksonFlag.value(); @@ -36,8 +32,6 @@ public class JacksonFlagTest { assertEquals("foo", value.string); assertEquals("3", value.dummy); - assertTrue(optionalJacksonFlag.value().isPresent()); - value = optionalJacksonFlag.value().get(); assertEquals(4, value.integer); assertEquals("foo", value.string); assertEquals("3", value.dummy); -- cgit v1.2.3