----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51010/#review145572 -----------------------------------------------------------
geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java (line 82) <https://reviews.apache.org/r/51010/#comment211869> Is this method needed? geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java (line 115) <https://reviews.apache.org/r/51010/#comment211874> Should we limit the index here or rely on the host.getVM for which index values are valid? If we do want to limit it in this rule, maybe break out 1 and 3 into constants? geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java (line 125) <https://reviews.apache.org/r/51010/#comment211875> If mcast port is set... what will happen? This checks to see if it's not set. geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java (line 129) <https://reviews.apache.org/r/51010/#comment211878> Should this be a SerializableRunnable? geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java (line 156) <https://reviews.apache.org/r/51010/#comment211877> Should this be a SerializableRunnable? It doesn't seem to matter what happens in this method, it will always return true and we are not actually using the return value anyways geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java (line 83) <https://reviews.apache.org/r/51010/#comment211879> I think these methods are self documenting, probably can remove the comments geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java (line 103) <https://reviews.apache.org/r/51010/#comment211864> Should we seperate this test into two tests? One for when the new node is created in the group and one for when it is out of the group? This test is named in a way that made me think every new node was in the group and should have the index, but vm3 is outside the group and verifies it does not get an index geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java (line 154) <https://reviews.apache.org/r/51010/#comment211865> What currently happens if we have RegionShortcut.REPLICATE? The index itself is just not created but everything else keeps on running? geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java (line 189) <https://reviews.apache.org/r/51010/#comment211863> Will this dir and clusterConfigDir get cleaned up after the test is run? Should this use a TemporaryFolder rule or something else? - Jason Huynh On Aug. 11, 2016, 10:34 p.m., anilkumar gingade wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51010/ > ----------------------------------------------------------- > > (Updated Aug. 11, 2016, 10:34 p.m.) > > > Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, Kirk > Lund, William Markito, nabarun nag, Dan Smith, and xiaojian zhou. > > > Repository: geode > > > Description > ------- > > Added test to validate Cluster configuration support for Lucene indexes. > > Added a new Rule to create Distributed System with test/custom > configuration...The "LocatorServerConfigurationRule" makes it easier to > create a Locator or cluster nodes with required configuration. > > > Diffs > ----- > > > geode-core/src/test/java/com/gemstone/gemfire/test/dunit/rules/LocatorServerConfigurationRule.java > PRE-CREATION > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51010/diff/ > > > Testing > ------- > > Run the newly added test. > > > Thanks, > > anilkumar gingade > >
