Remove nanoTime in global cluster randomization in favor of deriving the

seed from the main master seed. Removed shared cluster's seed entirely.

The problem here is that if you don't give cluster's seed then test times
fluctuate oddly, even for a fixed -Dtests.seed=... This shouldn't be the
case -- ideally, the test ran with the same master seed should reproduce
pretty much with the same execution time (and internal logic, obviously).

From the code point of view "global" variables are indeed a problem
because JUnit has no notion of before-suite hooks. And RandomizedRunner
doesn't support context accesses in static class initializers (this is
intentional because there is no way to determine when such initializers
will be executed). A workaround is to move such static global variables to
lazily-initialized methods and invoke them (once) in @BeforeClass hooks.
This commit is contained in:
Dawid Weiss 2014-02-24 13:46:35 +01:00 committed by Simon Willnauer
parent 83ae1bd55e
commit ad547eb7fb
5 changed files with 24 additions and 51 deletions

View File

@ -19,6 +19,8 @@
package org.elasticsearch.test;
import com.carrotsearch.hppc.ObjectArrayList;
import com.carrotsearch.randomizedtesting.RandomizedContext;
import com.carrotsearch.randomizedtesting.SeedUtils;
import com.google.common.base.Joiner;
import org.apache.lucene.util.AbstractRandomizedTest;
import org.elasticsearch.ElasticsearchIllegalArgumentException;
@ -64,6 +66,7 @@ import org.elasticsearch.search.SearchService;
import org.elasticsearch.test.client.RandomizingClient;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Ignore;
import java.io.IOException;
@ -76,7 +79,6 @@ import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import static org.elasticsearch.test.TestCluster.SHARED_CLUSTER_SEED;
import static org.elasticsearch.test.TestCluster.clusterName;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
@ -133,7 +135,6 @@ import static org.hamcrest.Matchers.equalTo;
* This class supports the following system properties (passed with -Dkey=value to the application)
* <ul>
* <li>-D{@value #TESTS_CLIENT_RATIO} - a double value in the interval [0..1] which defines the ration between node and transport clients used</li>
* <li>-D{@value TestCluster#TESTS_CLUSTER_SEED} - a random seed used to initialize the clusters random context.
* <li>-D{@value TestCluster#TESTS_ENABLE_MOCK_MODULES} - a boolean value to enable or disable mock modules. This is
* useful to test the system without asserting modules that to make sure they don't hide any bugs in production.</li>
* <li>-D{@value #INDEX_SEED_SETTING} - a random seed used to initialize the index random context.
@ -143,8 +144,7 @@ import static org.hamcrest.Matchers.equalTo;
@Ignore
@AbstractRandomizedTest.IntegrationTests
public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase {
private static final TestCluster GLOBAL_CLUSTER = new TestCluster(SHARED_CLUSTER_SEED, clusterName("shared", ElasticsearchTestCase.CHILD_VM_ID, SHARED_CLUSTER_SEED));
private static TestCluster GLOBAL_CLUSTER;
/**
* Key used to set the transport client ratio via the commandline -D{@value #TESTS_CLIENT_RATIO}
@ -169,6 +169,16 @@ public abstract class ElasticsearchIntegrationTest extends ElasticsearchTestCase
private static final Map<Class<?>, TestCluster> clusters = new IdentityHashMap<Class<?>, TestCluster>();
@BeforeClass
public final static void beforeClass() throws Exception {
// Initialize lazily. No need for volatiles/ CASs since each JVM runs at most one test
// suite at any given moment.
if (GLOBAL_CLUSTER == null) {
long masterSeed = SeedUtils.parseSeed(RandomizedContext.current().getRunnerSeedAsString());
GLOBAL_CLUSTER = new TestCluster(masterSeed, clusterName("shared", ElasticsearchTestCase.CHILD_VM_ID, masterSeed));
}
}
@Before
public final void before() throws IOException {
assert Thread.getDefaultUncaughtExceptionHandler() instanceof ElasticsearchUncaughtExceptionHandler;

View File

@ -37,7 +37,6 @@ import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
@ -91,17 +90,11 @@ public final class TestCluster implements Iterable<Client> {
private final ESLogger logger = Loggers.getLogger(getClass());
/**
* The random seed for the shared test cluster used in the current JVM.
*/
public static final long SHARED_CLUSTER_SEED = clusterSeed();
/**
* Key used to set the shared cluster random seed via the commandline -D{@value #TESTS_CLUSTER_SEED}
*/
public static final String TESTS_CLUSTER_SEED = "tests.cluster_seed";
/**
* Key used to set the shared cluster random seed via the commandline -D{@value #TESTS_CLUSTER_SEED}
* A boolean value to enable or disable mock modules. This is useful to test the
* system without asserting modules that to make sure they don't hide any bugs in
* production.
*
* @see ElasticsearchIntegrationTest
*/
public static final String TESTS_ENABLE_MOCK_MODULES = "tests.enable_mock_modules";
@ -118,14 +111,6 @@ public final class TestCluster implements Iterable<Client> {
static final int DEFAULT_MAX_NUM_NODES = 6;
private static long clusterSeed() {
String property = System.getProperty(TESTS_CLUSTER_SEED);
if (!Strings.hasLength(property)) {
return System.nanoTime();
}
return SeedUtils.parseSeed(property);
}
/* sorted map to make traverse order reproducible */
private final TreeMap<String, NodeAndClient> nodes = newTreeMap();

View File

@ -20,12 +20,10 @@ package org.elasticsearch.test.junit.listeners;
import com.carrotsearch.randomizedtesting.RandomizedContext;
import com.carrotsearch.randomizedtesting.ReproduceErrorMessageBuilder;
import com.carrotsearch.randomizedtesting.SeedUtils;
import com.carrotsearch.randomizedtesting.TraceFormatting;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.elasticsearch.test.TestCluster;
import org.junit.internal.AssumptionViolatedException;
@ -34,8 +32,6 @@ import org.junit.runner.notification.Failure;
import org.junit.runner.notification.RunListener;
import static com.carrotsearch.randomizedtesting.SysGlobals.SYSPROP_ITERATIONS;
import static org.elasticsearch.test.TestCluster.SHARED_CLUSTER_SEED;
import static org.elasticsearch.test.TestCluster.TESTS_CLUSTER_SEED;
/**
* A {@link RunListener} that emits to {@link System#err} a string with command
@ -66,11 +62,7 @@ public class ReproduceInfoPrinter extends RunListener {
final StringBuilder b = new StringBuilder();
b.append("FAILURE : ").append(d.getDisplayName()).append("\n");
b.append("REPRODUCE WITH : mvn test");
ReproduceErrorMessageBuilder builder = reproduceErrorMessageBuilder(b).appendAllOpts(failure.getDescription());
if (mustAppendClusterSeed(failure)) {
appendClusterSeed(builder);
}
reproduceErrorMessageBuilder(b).appendAllOpts(failure.getDescription());
b.append("\n");
b.append("Throwable:\n");
@ -81,14 +73,6 @@ public class ReproduceInfoPrinter extends RunListener {
logger.error(b.toString());
}
protected boolean mustAppendClusterSeed(Failure failure) {
return ElasticsearchIntegrationTest.class.isAssignableFrom(failure.getDescription().getTestClass());
}
protected void appendClusterSeed(ReproduceErrorMessageBuilder builder) {
builder.appendOpt(TESTS_CLUSTER_SEED, SeedUtils.formatSeed(SHARED_CLUSTER_SEED));
}
protected ReproduceErrorMessageBuilder reproduceErrorMessageBuilder(StringBuilder b) {
return new MavenMessageBuilder(b);
}

View File

@ -25,8 +25,8 @@ import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.test.junit.listeners.ReproduceInfoPrinter;
import org.elasticsearch.test.rest.ElasticsearchRestTests;
import org.elasticsearch.test.rest.junit.RestTestSuiteRunner.RunMode;
import org.junit.runner.Description;
import org.junit.runner.notification.Failure;
import java.util.Arrays;
@ -42,11 +42,6 @@ class RestReproduceInfoPrinter extends ReproduceInfoPrinter {
protected static final ESLogger logger = Loggers.getLogger(RestReproduceInfoPrinter.class);
@Override
protected boolean mustAppendClusterSeed(Failure failure) {
return isTestCluster();
}
private static boolean isTestCluster() {
return runMode() == RunMode.TEST_CLUSTER;
}

View File

@ -59,7 +59,6 @@ import java.util.regex.Pattern;
import static com.carrotsearch.randomizedtesting.SeedUtils.parseSeedChain;
import static com.carrotsearch.randomizedtesting.StandaloneRandomizedContext.*;
import static com.carrotsearch.randomizedtesting.SysGlobals.*;
import static org.elasticsearch.test.TestCluster.SHARED_CLUSTER_SEED;
import static org.elasticsearch.test.TestCluster.clusterName;
import static org.elasticsearch.test.rest.junit.DescriptionHelper.*;
import static org.hamcrest.Matchers.equalTo;
@ -184,8 +183,8 @@ public class RestTestSuiteRunner extends ParentRunner<RestTestCandidate> {
List<InetSocketAddress> addresses = Lists.newArrayList();
if (runMode == RunMode.TEST_CLUSTER) {
this.testCluster = new TestCluster(SHARED_CLUSTER_SEED, 1, 3,
clusterName("REST-tests", ElasticsearchTestCase.CHILD_VM_ID, SHARED_CLUSTER_SEED));
this.testCluster = new TestCluster(initialSeed, 1, 3,
clusterName("REST-tests", ElasticsearchTestCase.CHILD_VM_ID, initialSeed));
this.testCluster.beforeTest(runnerRandomness.getRandom(), 0.0f);
for (HttpServerTransport httpServerTransport : testCluster.getInstances(HttpServerTransport.class)) {
addresses.add(((InetSocketTransportAddress) httpServerTransport.boundAddress().publishAddress()).address());
@ -353,7 +352,7 @@ public class RestTestSuiteRunner extends ParentRunner<RestTestCandidate> {
return "elasticsearch REST Tests - not run";
}
if (runMode == RunMode.TEST_CLUSTER) {
return String.format(Locale.ROOT, "elasticsearch REST Tests - test cluster %s", SeedUtils.formatSeed(SHARED_CLUSTER_SEED));
return String.format(Locale.ROOT, "elasticsearch REST Tests - test cluster");
}
if (runMode == RunMode.EXTERNAL_CLUSTER) {
return String.format(Locale.ROOT, "elasticsearch REST Tests - external cluster %s", System.getProperty(REST_TESTS_MODE));