Skip to content

Commit 22f7b03

Browse files
author
Christoph Büscher
authored
Fix test reproducability in AbstractBuilderTestCase setup (#32403)
Currently AbstractBuilderTestCase generates certain random values in its `beforeTest()` method annotated with @before only the first time that a test method in the suite is run while initializing the serviceHolder that we use for the rest of the test. This changes the values of subsequent random values and has the effect that when running single methods from a test suite with "-Dtests.method=*", the random values it sees are different from when the same test method is run as part of the whole test suite. This makes it hard to use the reproduction lines logged on failure. This change runs the inialization of the serviceHolder and the randomization connected to it using the test runners master seed, so reproduction by running just one method is possible again. Closes #32400
1 parent de2897a commit 22f7b03

File tree

5 files changed

+37
-19
lines changed

5 files changed

+37
-19
lines changed

modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
8888
}
8989

9090
@Override
91-
protected Settings indexSettings() {
91+
protected Settings createTestIndexSettings() {
9292
return Settings.builder()
93-
.put(super.indexSettings())
93+
.put(super.createTestIndexSettings())
9494
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
9595
.build();
9696
}

modules/parent-join/src/test/java/org/elasticsearch/join/query/HasParentQueryBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
7373
}
7474

7575
@Override
76-
protected Settings indexSettings() {
76+
protected Settings createTestIndexSettings() {
7777
return Settings.builder()
78-
.put(super.indexSettings())
78+
.put(super.createTestIndexSettings())
7979
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
8080
.build();
8181
}

modules/parent-join/src/test/java/org/elasticsearch/join/query/ParentIdQueryBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
6262
}
6363

6464
@Override
65-
protected Settings indexSettings() {
65+
protected Settings createTestIndexSettings() {
6666
return Settings.builder()
67-
.put(super.indexSettings())
67+
.put(super.createTestIndexSettings())
6868
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
6969
.build();
7070
}

server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ public void testFieldsCannotBeSetToNull() {
233233

234234
public void testDefaultFieldParsing() throws IOException {
235235
assumeTrue("5.x behaves differently, so skip on non-6.x indices",
236-
indexVersionCreated.onOrAfter(Version.V_6_0_0_alpha1));
236+
indexSettings().getIndexVersionCreated().onOrAfter(Version.V_6_0_0_alpha1));
237237

238238
String query = randomAlphaOfLengthBetween(1, 10).toLowerCase(Locale.ROOT);
239239
String contentString = "{\n" +

test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
package org.elasticsearch.test;
2121

22+
import com.carrotsearch.randomizedtesting.RandomizedTest;
23+
import com.carrotsearch.randomizedtesting.SeedUtils;
24+
2225
import org.apache.lucene.index.IndexReader;
2326
import org.apache.lucene.util.Accountable;
2427
import org.elasticsearch.Version;
@@ -84,6 +87,7 @@
8487
import java.util.HashMap;
8588
import java.util.List;
8689
import java.util.Map;
90+
import java.util.concurrent.Callable;
8791
import java.util.concurrent.ExecutionException;
8892
import java.util.function.Function;
8993
import java.util.stream.Stream;
@@ -124,14 +128,12 @@ public abstract class AbstractBuilderTestCase extends ESTestCase {
124128
ALIAS_TO_CONCRETE_FIELD_NAME.put(GEO_POINT_ALIAS_FIELD_NAME, GEO_POINT_FIELD_NAME);
125129
}
126130

127-
protected static Version indexVersionCreated;
128-
129131
private static ServiceHolder serviceHolder;
130132
private static ServiceHolder serviceHolderWithNoType;
131133
private static int queryNameId = 0;
132134
private static Settings nodeSettings;
133135
private static Index index;
134-
private static Index indexWithNoType;
136+
private static long nowInMillis;
135137

136138
protected static Index getIndex() {
137139
return index;
@@ -152,7 +154,7 @@ public static void beforeClass() {
152154
.build();
153155

154156
index = new Index(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLength(10));
155-
indexWithNoType = new Index(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLength(10));
157+
nowInMillis = randomNonNegativeLong();
156158
}
157159

158160
@Override
@@ -173,15 +175,19 @@ protected static String createUniqueRandomName() {
173175
return queryName;
174176
}
175177

176-
protected Settings indexSettings() {
178+
protected Settings createTestIndexSettings() {
177179
// we have to prefer CURRENT since with the range of versions we support it's rather unlikely to get the current actually.
178-
indexVersionCreated = randomBoolean() ? Version.CURRENT
180+
Version indexVersionCreated = randomBoolean() ? Version.CURRENT
179181
: VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, Version.CURRENT);
180182
return Settings.builder()
181183
.put(IndexMetaData.SETTING_VERSION_CREATED, indexVersionCreated)
182184
.build();
183185
}
184186

187+
protected static IndexSettings indexSettings() {
188+
return serviceHolder.idxSettings;
189+
}
190+
185191
protected static String expectedFieldName(String builderFieldName) {
186192
return ALIAS_TO_CONCRETE_FIELD_NAME.getOrDefault(builderFieldName, builderFieldName);
187193
}
@@ -195,14 +201,24 @@ public static void afterClass() throws Exception {
195201
}
196202

197203
@Before
198-
public void beforeTest() throws IOException {
204+
public void beforeTest() throws Exception {
199205
if (serviceHolder == null) {
200-
serviceHolder = new ServiceHolder(nodeSettings, indexSettings(), getPlugins(), this, true);
206+
assert serviceHolderWithNoType == null;
207+
// we initialize the serviceHolder and serviceHolderWithNoType just once, but need some
208+
// calls to the randomness source during its setup. In order to not mix these calls with
209+
// the randomness source that is later used in the test method, we use the master seed during
210+
// this setup
211+
long masterSeed = SeedUtils.parseSeed(RandomizedTest.getContext().getRunnerSeedAsString());
212+
RandomizedTest.getContext().runWithPrivateRandomness(masterSeed, (Callable<Void>) () -> {
213+
serviceHolder = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis,
214+
AbstractBuilderTestCase.this, true);
215+
serviceHolderWithNoType = new ServiceHolder(nodeSettings, createTestIndexSettings(), getPlugins(), nowInMillis,
216+
AbstractBuilderTestCase.this, false);
217+
return null;
218+
});
201219
}
220+
202221
serviceHolder.clientInvocationHandler.delegate = this;
203-
if (serviceHolderWithNoType == null) {
204-
serviceHolderWithNoType = new ServiceHolder(nodeSettings, indexSettings(), getPlugins(), this, false);
205-
}
206222
serviceHolderWithNoType.clientInvocationHandler.delegate = this;
207223
}
208224

@@ -305,13 +321,15 @@ private static class ServiceHolder implements Closeable {
305321
private final BitsetFilterCache bitsetFilterCache;
306322
private final ScriptService scriptService;
307323
private final Client client;
308-
private final long nowInMillis = randomNonNegativeLong();
324+
private final long nowInMillis;
309325

310326
ServiceHolder(Settings nodeSettings,
311327
Settings indexSettings,
312328
Collection<Class<? extends Plugin>> plugins,
329+
long nowInMillis,
313330
AbstractBuilderTestCase testCase,
314331
boolean registerType) throws IOException {
332+
this.nowInMillis = nowInMillis;
315333
Environment env = InternalSettingsPreparer.prepareEnvironment(nodeSettings);
316334
PluginsService pluginsService;
317335
pluginsService = new PluginsService(nodeSettings, null, env.modulesFile(), env.pluginsFile(), plugins);

0 commit comments

Comments
 (0)