accumulo-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubb...@apache.org
Subject [accumulo] 03/03: ACCUMULO-4086 Make chooser classes more testable
Date Tue, 12 Sep 2017 02:24:49 GMT
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit 21fb47733416ec8690e364420c41d2e7b731cda7
Author: Christopher Tubbs <ctubbsii@apache.org>
AuthorDate: Mon Sep 11 21:48:41 2017 -0400

    ACCUMULO-4086 Make chooser classes more testable
    
    Improve the testability of PerTableVolumeChooser and
    PreferredVolumeChooser, and dramatically simplify their unit tests.
    
    PerTableVolumeChooser.choose becomes:
      getDelegate(env).choose(env, options);
    And PerTableVolumeChooserTest tests getDelegate(env)
    
    PreferredVolumeChooser.choose becomes:
      super.choose(env, getPreferredVolumes(env, options));
    And PreferredVolumesChooserTest tests getPreferredVolumes(env, options)
---
 .../accumulo/server/fs/PerTableVolumeChooser.java  |  50 ++--
 .../accumulo/server/fs/PreferredVolumeChooser.java |  33 ++-
 .../server/fs/PerTableVolumeChooserTest.java       | 281 +++++++--------------
 .../server/fs/PreferredVolumeChooserTest.java      | 251 ++++++++----------
 4 files changed, 233 insertions(+), 382 deletions(-)

diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
b/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
index 6eba22c..ab642d6 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/fs/PerTableVolumeChooser.java
@@ -58,22 +58,21 @@ public class PerTableVolumeChooser implements VolumeChooser {
   @Override
   public String choose(VolumeChooserEnvironment env, String[] options) throws VolumeChooserException
{
     log.trace("{}.choose", getClass().getSimpleName());
+    return getDelegateChooser(env).choose(env, options);
+  }
 
-    VolumeChooser delegateChooser;
+  // visible (not private) for testing
+  VolumeChooser getDelegateChooser(VolumeChooserEnvironment env) {
     switch (env.getScope()) {
       case INIT:
         // TODO should be possible to read from SiteConfiguration during init
-        log.warn("Not possible to determine delegate chooser at '{}' scope. Using all volumes.",
ChooserScope.INIT);
-        delegateChooser = randomChooser;
-        break;
+        log.warn("Not possible to determine delegate chooser at '{}' scope. Using {}.", ChooserScope.INIT,
RandomVolumeChooser.class.getName());
+        return randomChooser;
       case TABLE:
-        delegateChooser = getVolumeChooserForTable(env, loadConfFactory());
-        break;
+        return getVolumeChooserForTable(env, loadConfFactory());
       default:
-        delegateChooser = getVolumeChooserForScope(env, loadConfFactory());
-        break;
+        return getVolumeChooserForScope(env, loadConfFactory());
     }
-    return delegateChooser.choose(env, options);
   }
 
   private VolumeChooser getVolumeChooserForTable(VolumeChooserEnvironment env, ServerConfigurationFactory
confFactory) {
@@ -92,10 +91,15 @@ public class PerTableVolumeChooser implements VolumeChooser {
       throw new VolumeChooserException(msg);
     }
 
-    String context = tableConf.get(Property.TABLE_CLASSPATH); // can be null
+    String context = getTableContext(tableConf); // can be null
     return createVolumeChooser(context, clazz, TABLE_VOLUME_CHOOSER, env.getTableId(), tableSpecificChooserCache);
   }
 
+  // visible (not private) for testing
+  String getTableContext(TableConfiguration tableConf) {
+    return tableConf.get(Property.TABLE_CLASSPATH);
+  }
+
   private VolumeChooser getVolumeChooserForScope(VolumeChooserEnvironment env, ServerConfigurationFactory
confFactory) {
     ChooserScope scope = env.getScope();
     String property = getPropertyNameForScope(scope);
@@ -142,22 +146,22 @@ public class PerTableVolumeChooser implements VolumeChooser {
       if (previousChooser != null && previousChooser.getClass().getName().equals(className))
{
         // no change; return the old one
         return previousChooser;
+      } else if (previousChooser == null) {
+        // TODO stricter definition of when the updated property is used, ref ACCUMULO-3412
+        // don't log change if this is the first use
+        log.trace("Change detected for {} for {}", property, key);
+      }
+      try {
+        return ConfigurationTypeHelper.getClassInstance(context, className, VolumeChooser.class);
+      } catch (ClassNotFoundException | InstantiationException | IllegalAccessException |
IOException e) {
+        String msg = "Failed to create instance for " + key + " configured to use " + className
+ " via " + property;
+        throw new VolumeChooserException(msg, e);
       }
-      // TODO stricter definition of when the updated property is used, ref ACCUMULO-3412
-        if (previousChooser == null) {
-          // don't log change if this is the first use
-          log.trace("Change detected for {} for {}", property, key);
-        }
-        try {
-          return ConfigurationTypeHelper.getClassInstance(context, className, VolumeChooser.class);
-        } catch (ClassNotFoundException | InstantiationException | IllegalAccessException
| IOException e) {
-          String msg = "Failed to create instance for " + key + " configured to use " + className
+ " via " + property;
-          throw new VolumeChooserException(msg, e);
-        }
-      });
+    });
   }
 
-  private ServerConfigurationFactory loadConfFactory() {
+  // visible (not private) for testing
+  ServerConfigurationFactory loadConfFactory() {
     // This local variable is an intentional component of the single-check idiom.
     ServerConfigurationFactory localConf = lazyConfFactory;
     if (localConf == null) {
diff --git a/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java
b/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java
index 8bf5f2a..e1a9b12 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/fs/PreferredVolumeChooser.java
@@ -53,29 +53,27 @@ public class PreferredVolumeChooser extends RandomVolumeChooser {
   @Override
   public String choose(VolumeChooserEnvironment env, String[] options) throws VolumeChooserException
{
     log.trace("{}.choose", getClass().getSimpleName());
+    // Randomly choose the volume from the preferred volumes
+    String choice = super.choose(env, getPreferredVolumes(env, options));
+    log.trace("Choice = {}", choice);
+    return choice;
+  }
 
-    Set<String> preferredVolumes;
+  // visible (not private) for testing
+  String[] getPreferredVolumes(VolumeChooserEnvironment env, String[] options) {
     switch (env.getScope()) {
       case INIT:
         // TODO should be possible to read from SiteConfiguration during init
         log.warn("Not possible to determine preferred volumes at '{}' scope. Using all volumes.",
ChooserScope.INIT);
-        return super.choose(env, options);
+        return options;
       case TABLE:
-        preferredVolumes = getPreferredVolumesForTable(env, loadConfFactory(), options);
-        break;
+        return getPreferredVolumesForTable(env, loadConfFactory(), options);
       default:
-        preferredVolumes = getPreferredVolumesForScope(env, loadConfFactory(), options);
-        break;
+        return getPreferredVolumesForScope(env, loadConfFactory(), options);
     }
-
-    // Randomly choose the volume from the preferred volumes
-    String choice = super.choose(env, preferredVolumes.toArray(new String[preferredVolumes.size()]));
-    log.trace("Choice = {}", choice);
-
-    return choice;
   }
 
-  private Set<String> getPreferredVolumesForTable(VolumeChooserEnvironment env, ServerConfigurationFactory
confFactory, String[] options) {
+  private String[] getPreferredVolumesForTable(VolumeChooserEnvironment env, ServerConfigurationFactory
confFactory, String[] options) {
     log.trace("Looking up property {} + for Table id: {}", TABLE_PREFERRED_VOLUMES, env.getTableId());
 
     final TableConfiguration tableConf = confFactory.getTableConfiguration(env.getTableId());
@@ -96,7 +94,7 @@ public class PreferredVolumeChooser extends RandomVolumeChooser {
     return parsePreferred(TABLE_PREFERRED_VOLUMES, preferredVolumes, options);
   }
 
-  private Set<String> getPreferredVolumesForScope(VolumeChooserEnvironment env, ServerConfigurationFactory
confFactory, String[] options) {
+  private String[] getPreferredVolumesForScope(VolumeChooserEnvironment env, ServerConfigurationFactory
confFactory, String[] options) {
     ChooserScope scope = env.getScope();
     String property = getPropertyNameForScope(scope);
     log.trace("Looking up property {} for scope: {}", property, scope);
@@ -122,7 +120,7 @@ public class PreferredVolumeChooser extends RandomVolumeChooser {
     return parsePreferred(property, preferredVolumes, options);
   }
 
-  private Set<String> parsePreferred(String property, String preferredVolumes, String[]
options) {
+  private String[] parsePreferred(String property, String preferredVolumes, String[] options)
{
     log.trace("Found {} = {}", property, preferredVolumes);
 
     Set<String> preferred = Arrays.stream(StringUtils.split(preferredVolumes, ',')).map(String::trim).collect(Collectors.toSet());
@@ -137,10 +135,11 @@ public class PreferredVolumeChooser extends RandomVolumeChooser {
       throw new VolumeChooserException(msg);
     }
 
-    return preferred;
+    return preferred.toArray(new String[preferred.size()]);
   }
 
-  private ServerConfigurationFactory loadConfFactory() {
+  // visible (not private) for testing
+  ServerConfigurationFactory loadConfFactory() {
     // Get the current table's properties, and find the preferred volumes property
     // This local variable is an intentional component of the single-check idiom.
     ServerConfigurationFactory localConf = lazyConfFactory;
diff --git a/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java
b/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java
index 3d1a3f1..6f6ba52 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/fs/PerTableVolumeChooserTest.java
@@ -16,265 +16,164 @@
  */
 package org.apache.accumulo.server.fs;
 
-import java.lang.reflect.Field;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
 
 import org.apache.accumulo.core.client.impl.Table;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
-import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.server.conf.ServerConfigurationFactory;
 import org.apache.accumulo.server.conf.TableConfiguration;
 import org.apache.accumulo.server.fs.VolumeChooser.VolumeChooserException;
 import org.apache.accumulo.server.fs.VolumeChooserEnvironment.ChooserScope;
-import org.easymock.EasyMock;
-import org.easymock.IExpectationSetters;
-import org.junit.Assert;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import com.google.common.collect.Sets;
-
 public class PerTableVolumeChooserTest {
-  private static final int REQUIRED_NUMBER_TRIES = 20; // times to call choose for likely
exercising of each preferred volume
-  private static final String[] ALL_OPTIONS = new String[] {"1", "2", "3"};
-  public static final String INVALID_CHOOSER_CLASSNAME = "MysteriousVolumeChooser";
-  private ServerConfigurationFactory mockedServerConfigurationFactory;
-  private TableConfiguration mockedTableConfiguration;
-  private PerTableVolumeChooser perTableVolumeChooser;
-  private AccumuloConfiguration mockedAccumuloConfiguration;
+
+  private ServerConfigurationFactory confFactory;
+  private TableConfiguration tableConf;
+  private PerTableVolumeChooser chooser;
+  private AccumuloConfiguration systemConf;
+
+  public static class MockChooser1 extends RandomVolumeChooser {}
+
+  public static class MockChooser2 extends RandomVolumeChooser {}
 
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
   @Before
   public void before() throws Exception {
-    perTableVolumeChooser = new PerTableVolumeChooser();
-
-    mockedServerConfigurationFactory = EasyMock.createMock(ServerConfigurationFactory.class);
-    Field field = perTableVolumeChooser.getClass().getDeclaredField("lazyConfFactory");
-    field.setAccessible(true);
-    field.set(perTableVolumeChooser, mockedServerConfigurationFactory);
-
-    mockedTableConfiguration = EasyMock.createMock(TableConfiguration.class);
-    mockedAccumuloConfiguration = EasyMock.createMock(AccumuloConfiguration.class);
-    EasyMock.expect(mockedServerConfigurationFactory.getTableConfiguration(EasyMock.<Table.ID>
anyObject())).andReturn(mockedTableConfiguration).anyTimes();
-    EasyMock.expect(mockedServerConfigurationFactory.getSystemConfiguration()).andReturn(mockedAccumuloConfiguration).anyTimes();
-    EasyMock.expect(mockedTableConfiguration.get(Property.TABLE_CLASSPATH)).andReturn(null).anyTimes();
-  }
-
-  private IExpectationSetters<String> expectDefaultScope(String className) {
-    return expectScope(ChooserScope.DEFAULT, className);
-  }
+    confFactory = createStrictMock(ServerConfigurationFactory.class);
 
-  private IExpectationSetters<String> expectLoggerScope(String className) {
-    return expectScope(ChooserScope.LOGGER, className);
-  }
-
-  private IExpectationSetters<String> expectScope(ChooserScope scope, String className)
{
-    return EasyMock.expect(mockedAccumuloConfiguration.get(PerTableVolumeChooser.getPropertyNameForScope(scope))).andReturn(className);
-  }
-
-  private IExpectationSetters<String> expectTableChooser(String className) {
-    return EasyMock.expect(mockedTableConfiguration.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(className);
-  }
-
-  private Set<String> chooseRepeatedlyForTable() {
-    VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(Table.ID.of("h"));
-    Set<String> results = new HashSet<>();
-    for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) {
-      results.add(perTableVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS));
-    }
-    return results;
-  }
-
-  public static class VolumeChooserAlwaysOne extends VolumeChooserForFixedVolume {
-    public VolumeChooserAlwaysOne() {
-      super("1");
-    }
-  }
-
-  public static class VolumeChooserAlwaysTwo extends VolumeChooserForFixedVolume {
-    public VolumeChooserAlwaysTwo() {
-      super("2");
-    }
-  }
-
-  public static class VolumeChooserAlwaysThree extends VolumeChooserForFixedVolume {
-    public VolumeChooserAlwaysThree() {
-      super("3");
-    }
-  }
-
-  public static class VolumeChooserForFixedVolume implements VolumeChooser {
-    private final String onlyValidOption;
-
-    public VolumeChooserForFixedVolume(String fixedVolume) {
-      onlyValidOption = fixedVolume;
-    }
+    chooser = new PerTableVolumeChooser() {
+      @Override
+      ServerConfigurationFactory loadConfFactory() {
+        return confFactory;
+      }
 
-    @Override
-    public String choose(VolumeChooserEnvironment env, String[] options) {
-      for (String option : options) {
-        if (onlyValidOption.equals(option)) {
-          return onlyValidOption;
-        }
+      @Override
+      String getTableContext(TableConfiguration tableConf) {
+        return null;
       }
-      return null;
-    }
-  }
+    };
 
-  private Set<String> chooseRepeatedlyForLogger() {
-    return chooseRepeatedlyForScope(ChooserScope.LOGGER);
+    tableConf = createStrictMock(TableConfiguration.class);
+    systemConf = createStrictMock(AccumuloConfiguration.class);
+    expect(confFactory.getTableConfiguration(anyObject())).andReturn(tableConf).anyTimes();
+    expect(confFactory.getSystemConfiguration()).andReturn(systemConf).anyTimes();
   }
 
-  private Set<String> chooseRepeatedlyForScope(ChooserScope scope) {
-    VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(scope);
-    Set<String> results = new HashSet<>();
-
-    for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) {
-      results.add(perTableVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS));
-    }
-    return results;
+  @After
+  public void after() throws Exception {
+    verify(confFactory, tableConf, systemConf);
   }
 
-  @Test
-  public void testInitScope() throws Exception {
-    VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(ChooserScope.INIT);
-
-    Set<String> results = new HashSet<>();
-    for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) {
-      results.add(perTableVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS));
-    }
-
-    Assert.assertEquals(Sets.newHashSet(Arrays.asList(ALL_OPTIONS)), results);
+  private VolumeChooser getTableDelegate() {
+    VolumeChooserEnvironment env = new VolumeChooserEnvironment(Table.ID.of("testTable"));
+    return chooser.getDelegateChooser(env);
   }
 
-  @Test
-  public void testTableConfig() throws Exception {
-    expectTableChooser(VolumeChooserAlwaysTwo.class.getName()).atLeastOnce();
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-
-    Set<String> results = chooseRepeatedlyForTable();
-
-    EasyMock.verify(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-    Assert.assertEquals(Sets.newHashSet(Arrays.asList("2")), results);
+  private VolumeChooser getDelegate(ChooserScope scope) {
+    VolumeChooserEnvironment env = new VolumeChooserEnvironment(scope);
+    return chooser.getDelegateChooser(env);
   }
 
   @Test
-  public void testTableMisconfigured() throws Exception {
-    expectDefaultScope(VolumeChooserAlwaysOne.class.getName());
-    expectTableChooser(INVALID_CHOOSER_CLASSNAME);
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+  public void testInitScopeSelectsRandomChooser() throws Exception {
+    replay(confFactory, tableConf, systemConf);
+    VolumeChooser delegate = getDelegate(ChooserScope.INIT);
+    assertSame(RandomVolumeChooser.class, delegate.getClass());
   }
 
   @Test
-  public void testTableMissing() throws Exception {
-    expectDefaultScope(null);
-    expectTableChooser(null);
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeUsingTableProperty() throws Exception {
+    expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(MockChooser1.class.getName());
+    replay(confFactory, tableConf, systemConf);
 
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    VolumeChooser delegate = getTableDelegate();
+    assertSame(MockChooser1.class, delegate.getClass());
   }
 
   @Test
-  public void testTableMisconfiguredAndDefaultEmpty() throws Exception {
-    expectDefaultScope("");
-    expectTableChooser(INVALID_CHOOSER_CLASSNAME);
+  public void testTableScopeUsingDefaultScopeProperty() throws Exception {
+    expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(null).once();
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(MockChooser2.class.getName()).once();
+    replay(confFactory, tableConf, systemConf);
 
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    VolumeChooser delegate = getTableDelegate();
+    assertSame(MockChooser2.class, delegate.getClass());
   }
 
   @Test
-  public void testTableEmptyConfig() throws Exception {
-    expectDefaultScope("");
-    expectTableChooser("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeWithNoConfig() throws Exception {
+    expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(null).once();
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    getTableDelegate();
+    fail("should not reach");
   }
 
   @Test
-  public void testTableAndDefaultEmpty() throws Exception {
-    expectDefaultScope("");
-    expectTableChooser("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeWithBadDelegate() throws Exception {
+    expect(tableConf.get(PerTableVolumeChooser.TABLE_VOLUME_CHOOSER)).andReturn(null).once();
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("not
a valid class name").once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    getTableDelegate();
+    fail("should not reach");
   }
 
   @Test
-  public void testScopeConfig() throws Exception {
-    expectLoggerScope(VolumeChooserAlwaysOne.class.getName()).atLeastOnce();
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeUsingLoggerProperty() throws Exception {
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(MockChooser1.class.getName()).once();
+    replay(confFactory, tableConf, systemConf);
 
-    Set<String> results = chooseRepeatedlyForLogger();
-
-    EasyMock.verify(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
-    Assert.assertEquals(Sets.newHashSet(Arrays.asList("1")), results);
+    VolumeChooser delegate = getDelegate(ChooserScope.LOGGER);
+    assertSame(MockChooser1.class, delegate.getClass());
   }
 
   @Test
-  public void testScopeMisconfigured() throws Exception {
-    expectDefaultScope(VolumeChooserAlwaysThree.class.getName());
-    expectLoggerScope(INVALID_CHOOSER_CLASSNAME);
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeUsingDefaultProperty() throws Exception {
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once();
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(MockChooser2.class.getName()).once();
+    replay(confFactory, tableConf, systemConf);
 
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    VolumeChooser delegate = getDelegate(ChooserScope.LOGGER);
+    assertSame(MockChooser2.class, delegate.getClass());
   }
 
   @Test
-  public void testScopeMissing() throws Exception {
-    expectLoggerScope(null);
-    expectDefaultScope(null);
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeWithNoConfig() throws Exception {
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once();
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    getDelegate(ChooserScope.LOGGER);
+    fail("should not reach");
   }
 
   @Test
-  public void testScopeMisconfiguredAndDefaultEmpty() throws Exception {
-    expectDefaultScope("");
-    expectTableChooser("");
-    expectLoggerScope(INVALID_CHOOSER_CLASSNAME);
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
-
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
-  }
-
-  @Test
-  public void testScopeAndDefaultBothEmpty() throws Exception {
-    expectDefaultScope("");
-    expectLoggerScope("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeWithBadDelegate() throws Exception {
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once();
+    expect(systemConf.get(PerTableVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("not
a valid class name").once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    getDelegate(ChooserScope.LOGGER);
+    fail("should not reach");
   }
 
 }
diff --git a/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java
b/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java
index 440c628..2110b40 100644
--- a/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java
+++ b/server/base/src/test/java/org/apache/accumulo/server/fs/PreferredVolumeChooserTest.java
@@ -16,10 +16,16 @@
  */
 package org.apache.accumulo.server.fs;
 
-import java.lang.reflect.Field;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.fail;
+
 import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
 
 import org.apache.accumulo.core.client.impl.Table;
 import org.apache.accumulo.core.conf.AccumuloConfiguration;
@@ -27,224 +33,167 @@ import org.apache.accumulo.server.conf.ServerConfigurationFactory;
 import org.apache.accumulo.server.conf.TableConfiguration;
 import org.apache.accumulo.server.fs.VolumeChooser.VolumeChooserException;
 import org.apache.accumulo.server.fs.VolumeChooserEnvironment.ChooserScope;
-import org.easymock.EasyMock;
-import org.easymock.IExpectationSetters;
-import org.junit.Assert;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import com.google.common.collect.Sets;
-
 public class PreferredVolumeChooserTest {
-  private static final int REQUIRED_NUMBER_TRIES = 20; // times to call choose for likely
exercising of each preferred volume
+
   private static final String[] ALL_OPTIONS = new String[] {"1", "2", "3"};
-  private ServerConfigurationFactory mockedServerConfigurationFactory;
-  private TableConfiguration mockedTableConfiguration;
-  private PreferredVolumeChooser preferredVolumeChooser;
-  private AccumuloConfiguration mockedAccumuloConfiguration;
+
+  private ServerConfigurationFactory confFactory;
+  private TableConfiguration tableConf;
+  private PreferredVolumeChooser chooser;
+  private AccumuloConfiguration systemConf;
 
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
   @Before
   public void before() throws Exception {
-    preferredVolumeChooser = new PreferredVolumeChooser();
-
-    mockedServerConfigurationFactory = EasyMock.createMock(ServerConfigurationFactory.class);
-    Field field = preferredVolumeChooser.getClass().getDeclaredField("lazyConfFactory");
-    field.setAccessible(true);
-    field.set(preferredVolumeChooser, mockedServerConfigurationFactory);
+    confFactory = createStrictMock(ServerConfigurationFactory.class);
 
-    mockedTableConfiguration = EasyMock.createMock(TableConfiguration.class);
-    mockedAccumuloConfiguration = EasyMock.createMock(AccumuloConfiguration.class);
-    EasyMock.expect(mockedServerConfigurationFactory.getTableConfiguration(EasyMock.<Table.ID>
anyObject())).andReturn(mockedTableConfiguration).anyTimes();
-    EasyMock.expect(mockedServerConfigurationFactory.getSystemConfiguration()).andReturn(mockedAccumuloConfiguration).anyTimes();
-  }
-
-  private IExpectationSetters<String> expectTableVolumes(String configuredVolumes)
{
-    return EasyMock.expect(mockedTableConfiguration.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(configuredVolumes);
-  }
-
-  private IExpectationSetters<String> expectDefaultScope(String configuredVolumes)
{
-    return expectScope(ChooserScope.DEFAULT, configuredVolumes);
-  }
+    chooser = new PreferredVolumeChooser() {
+      @Override
+      ServerConfigurationFactory loadConfFactory() {
+        return confFactory;
+      }
+    };
 
-  private IExpectationSetters<String> expectLoggerScope(String configuredVolumes) {
-    return expectScope(ChooserScope.LOGGER, configuredVolumes);
+    tableConf = createStrictMock(TableConfiguration.class);
+    systemConf = createStrictMock(AccumuloConfiguration.class);
+    expect(confFactory.getTableConfiguration(anyObject())).andReturn(tableConf).anyTimes();
+    expect(confFactory.getSystemConfiguration()).andReturn(systemConf).anyTimes();
   }
 
-  private IExpectationSetters<String> expectScope(ChooserScope scope, String configuredVolumes)
{
-    return EasyMock.expect(mockedAccumuloConfiguration.get(PreferredVolumeChooser.getPropertyNameForScope(scope))).andReturn(configuredVolumes);
+  @After
+  public void after() throws Exception {
+    verify(confFactory, tableConf, systemConf);
   }
 
-  private Set<String> chooseRepeatedlyForTable() {
-    VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(Table.ID.of("h"));
-    Set<String> results = new HashSet<>();
-    for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) {
-      results.add(preferredVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS));
-    }
-    return results;
+  private String[] chooseForTable() {
+    VolumeChooserEnvironment env = new VolumeChooserEnvironment(Table.ID.of("testTable"));
+    return chooser.getPreferredVolumes(env, ALL_OPTIONS);
   }
 
-  private Set<String> chooseRepeatedlyForLogger() {
-    return chooseRepeatedlyForScope(ChooserScope.LOGGER);
-  }
-
-  private Set<String> chooseRepeatedlyForScope(ChooserScope scope) {
-    VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(scope);
-    Set<String> results = new HashSet<>();
-
-    for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) {
-      results.add(preferredVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS));
-    }
-    return results;
+  private String[] choose(ChooserScope scope) {
+    VolumeChooserEnvironment env = new VolumeChooserEnvironment(scope);
+    return chooser.getPreferredVolumes(env, ALL_OPTIONS);
   }
 
   @Test
-  public void testEmptyEnvUsesRandomChooser() throws Exception {
-    VolumeChooserEnvironment volumeChooserEnvironment = new VolumeChooserEnvironment(ChooserScope.INIT);
-    Set<String> results = new HashSet<>();
-    for (int i = 0; i < REQUIRED_NUMBER_TRIES; i++) {
-      results.add(preferredVolumeChooser.choose(volumeChooserEnvironment, ALL_OPTIONS));
-    }
-
-    Assert.assertEquals(Sets.newHashSet(Arrays.asList(ALL_OPTIONS)), results);
-  }
-
-  @Test
-  public void testTableConfig() throws Exception {
-    expectDefaultScope(null).anyTimes();
-    expectTableVolumes("1,2").atLeastOnce();
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-
-    Set<String> results = chooseRepeatedlyForTable();
-
-    EasyMock.verify(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-    Assert.assertEquals(Sets.newHashSet(Arrays.asList("1", "2")), results);
+  public void testInitScopeSelectsRandomlyFromAll() throws Exception {
+    replay(confFactory, tableConf, systemConf);
+    String[] volumes = choose(ChooserScope.INIT);
+    assertSame(ALL_OPTIONS, volumes);
   }
 
   @Test
-  public void testTableMisconfigured() throws Exception {
-    expectDefaultScope("1,3");
-    expectTableVolumes("4");
+  public void testTableScopeUsingTableProperty() throws Exception {
+    expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn("2,1");
+    replay(confFactory, tableConf, systemConf);
 
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
-
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    String[] volumes = chooseForTable();
+    Arrays.sort(volumes);
+    assertArrayEquals(new String[] {"1", "2"}, volumes);
   }
 
   @Test
-  public void testTableMissing() throws Exception {
-    expectDefaultScope("");
-    expectTableVolumes(null);
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeUsingDefaultScopeProperty() throws Exception {
+    expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(null).once();
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("3,2").once();
+    replay(confFactory, tableConf, systemConf);
 
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    String[] volumes = chooseForTable();
+    Arrays.sort(volumes);
+    assertArrayEquals(new String[] {"2", "3"}, volumes);
   }
 
   @Test
-  public void testTableEmptyConfig() throws Exception {
-    expectDefaultScope(null);
-    expectTableVolumes("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeWithNoConfig() throws Exception {
+    expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(null).once();
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    chooseForTable();
+    fail("should not reach");
   }
 
   @Test
-  public void testTableMisconfiguredAndDefaultEmpty() throws Exception {
-    expectDefaultScope("");
-    expectTableVolumes("4");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeWithEmptySet() throws Exception {
+    expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(",").once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    chooseForTable();
+    fail("should not reach");
   }
 
   @Test
-  public void testTableAndDefaultEmpty() throws Exception {
-    expectDefaultScope("");
-    expectTableVolumes("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedTableConfiguration, mockedAccumuloConfiguration);
+  public void testTableScopeWithUnrecognizedVolumes() throws Exception {
+    expect(tableConf.get(PreferredVolumeChooser.TABLE_PREFERRED_VOLUMES)).andReturn(null).once();
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("4").once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForTable();
+    chooseForTable();
+    fail("should not reach");
   }
 
   @Test
-  public void testScopeConfig() throws Exception {
-    expectLoggerScope("1,2").atLeastOnce();
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeUsingLoggerProperty() throws Exception {
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn("2,1").once();
+    replay(confFactory, tableConf, systemConf);
 
-    Set<String> results = chooseRepeatedlyForLogger();
-
-    EasyMock.verify(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
-    Assert.assertEquals(Sets.newHashSet(Arrays.asList("1", "2")), results);
+    String[] volumes = choose(ChooserScope.LOGGER);
+    Arrays.sort(volumes);
+    assertArrayEquals(new String[] {"1", "2"}, volumes);
   }
 
   @Test
-  public void testScopeMisconfigured() throws Exception {
-    expectDefaultScope("1,3");
-    expectLoggerScope("4");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeUsingDefaultProperty() throws Exception {
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once();
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("3,2").once();
+    replay(confFactory, tableConf, systemConf);
 
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    String[] volumes = choose(ChooserScope.LOGGER);
+    Arrays.sort(volumes);
+    assertArrayEquals(new String[] {"2", "3"}, volumes);
   }
 
   @Test
-  public void testScopeMissing() throws Exception {
-    expectDefaultScope("").atLeastOnce();
-    expectLoggerScope(null).atLeastOnce();
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeWithNoConfig() throws Exception {
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once();
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn(null).once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    choose(ChooserScope.LOGGER);
+    fail("should not reach");
   }
 
   @Test
-  public void testScopeMisconfiguredAndDefaultEmpty() throws Exception {
-    expectDefaultScope("");
-    expectLoggerScope("4");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeWithEmptySet() throws Exception {
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(",").once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    choose(ChooserScope.LOGGER);
+    fail("should not reach");
   }
 
   @Test
-  public void testScopeAndDefaultBothEmpty() throws Exception {
-    expectDefaultScope("");
-    expectLoggerScope("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
+  public void testLoggerScopeWithUnrecognizedVolumes() throws Exception {
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.LOGGER))).andReturn(null).once();
+    expect(systemConf.get(PreferredVolumeChooser.getPropertyNameForScope(ChooserScope.DEFAULT))).andReturn("4").once();
+    replay(confFactory, tableConf, systemConf);
 
     thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
+    choose(ChooserScope.LOGGER);
+    fail("should not reach");
   }
 
-  @Test
-  public void testScopeEmptyConfig() throws Exception {
-    expectDefaultScope("");
-    expectLoggerScope("");
-
-    EasyMock.replay(mockedServerConfigurationFactory, mockedAccumuloConfiguration);
-
-    thrown.expect(VolumeChooserException.class);
-    chooseRepeatedlyForLogger();
-  }
 }

-- 
To stop receiving notification emails like this one, please contact
"commits@accumulo.apache.org" <commits@accumulo.apache.org>.

Mime
View raw message