Skip to content

Commit f01c12b

Browse files
DaveCTurnerDaveCTurner
committed
Move systemPropertiesOverrideTest to separate source set
Today `:server:systemPropertiesOverrideTest` uses the same source set as `:server:test`, but runs a disjoint set of tests via includes & excludes. This forces every developer to choose in their IDE between the two tasks when running any unit test in `:server` even though there's only one appropriate task to run for any particular test. This commit moves the serverless-specific unit tests into their own source set to remove this friction in the development flow.
1 parent c92eef8 commit f01c12b

File tree

6 files changed

+197
-117
lines changed

6 files changed

+197
-117
lines changed

‎qa/serverless-system-properties/build.gradle‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
import org.apache.tools.ant.filters.ReplaceTokens
11+
12+
apply plugin: 'elasticsearch.build'
13+
14+
dependencies {
15+
testImplementation project(':test:framework')
16+
testImplementation project(':server')
17+
}
18+
19+
tasks.named("test").configure {
20+
systemProperty 'es.stateless.allow.index.refresh_interval.override', 'true'
21+
// systemProperty 'es.serverless_transport', 'true'
22+
}

‎server/src/test/java/org/elasticsearch/index/IndexSettingsOverrideTests.java‎ renamed to ‎qa/serverless-system-properties/src/test/java/org/elasticsearch/index/IndexSettingsOverrideTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static IndexMetadata newIndexMeta(String name, Settings indexSettings) {
2727
}
2828

2929
public void testStatelessMinRefreshIntervalOverride() {
30-
assumeTrue(
30+
assertTrue(
3131
"This test depends on system property configured in build.gradle",
3232
Booleans.parseBoolean(
3333
System.getProperty(IndexSettings.RefreshIntervalValidator.STATELESS_ALLOW_INDEX_REFRESH_INTERVAL_OVERRIDE, "false")

‎qa/serverless-system-properties/src/test/java/org/elasticsearch/transport/ServerlessTransportHandshakeTests.java‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.transport;
11+
12+
import org.elasticsearch.Build;
13+
import org.elasticsearch.TransportVersion;
14+
import org.elasticsearch.cluster.node.DiscoveryNodeUtils;
15+
import org.elasticsearch.cluster.node.VersionInformation;
16+
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
17+
import org.elasticsearch.common.network.NetworkService;
18+
import org.elasticsearch.common.settings.Settings;
19+
import org.elasticsearch.common.util.PageCacheRecycler;
20+
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
21+
import org.elasticsearch.test.ESTestCase;
22+
import org.elasticsearch.test.transport.MockTransportService;
23+
import org.elasticsearch.threadpool.TestThreadPool;
24+
import org.elasticsearch.threadpool.ThreadPool;
25+
import org.elasticsearch.transport.netty4.Netty4Transport;
26+
import org.elasticsearch.transport.netty4.SharedGroupFactory;
27+
import org.junit.After;
28+
import org.junit.AfterClass;
29+
import org.junit.BeforeClass;
30+
31+
import java.util.ArrayList;
32+
import java.util.Collections;
33+
import java.util.List;
34+
import java.util.concurrent.TimeUnit;
35+
36+
import static java.util.Collections.emptySet;
37+
import static org.elasticsearch.transport.AbstractSimpleTransportTestCase.IGNORE_DESERIALIZATION_ERRORS_SETTING;
38+
39+
public class ServerlessTransportHandshakeTests extends ESTestCase {
40+
41+
private static ThreadPool threadPool;
42+
43+
@BeforeClass
44+
public static void startThreadPool() {
45+
threadPool = new TestThreadPool(ServerlessTransportHandshakeTests.class.getSimpleName());
46+
}
47+
48+
private final List<TransportService> transportServices = new ArrayList<>();
49+
50+
private TransportService startServices(String nodeNameAndId, Settings settings, TransportInterceptor transportInterceptor) {
51+
TcpTransport transport = new Netty4Transport(
52+
settings,
53+
TransportVersion.current(),
54+
threadPool,
55+
new NetworkService(Collections.emptyList()),
56+
PageCacheRecycler.NON_RECYCLING_INSTANCE,
57+
new NamedWriteableRegistry(Collections.emptyList()),
58+
new NoneCircuitBreakerService(),
59+
new SharedGroupFactory(settings)
60+
);
61+
TransportService transportService = new MockTransportService(
62+
settings,
63+
transport,
64+
threadPool,
65+
transportInterceptor,
66+
(boundAddress) -> DiscoveryNodeUtils.builder(nodeNameAndId)
67+
.name(nodeNameAndId)
68+
.address(boundAddress.publishAddress())
69+
.roles(emptySet())
70+
.version(VersionInformation.CURRENT)
71+
.build(),
72+
null,
73+
Collections.emptySet(),
74+
nodeNameAndId
75+
);
76+
transportService.start();
77+
transportService.acceptIncomingRequests();
78+
transportServices.add(transportService);
79+
return transportService;
80+
}
81+
82+
@After
83+
public void tearDown() throws Exception {
84+
for (TransportService transportService : transportServices) {
85+
transportService.close();
86+
}
87+
super.tearDown();
88+
}
89+
90+
@AfterClass
91+
public static void terminateThreadPool() {
92+
ThreadPool.terminate(threadPool, 30, TimeUnit.SECONDS);
93+
// since static must set to null to be eligible for collection
94+
threadPool = null;
95+
}
96+
97+
public void testAcceptsMismatchedServerlessBuildHash() {
98+
assumeTrue("Current build needs to be a snapshot", Build.current().isSnapshot());
99+
final var transportInterceptorA = new BuildHashModifyingTransportInterceptor();
100+
final var transportInterceptorB = new BuildHashModifyingTransportInterceptor();
101+
final Settings settings = Settings.builder()
102+
.put("cluster.name", "a")
103+
.put(IGNORE_DESERIALIZATION_ERRORS_SETTING.getKey(), true) // suppress assertions to test production error-handling
104+
.build();
105+
final TransportService transportServiceA = startServices("TS_A", settings, transportInterceptorA);
106+
final TransportService transportServiceB = startServices("TS_B", settings, transportInterceptorB);
107+
AbstractSimpleTransportTestCase.connectToNode(transportServiceA, transportServiceB.getLocalNode(), TestProfiles.LIGHT_PROFILE);
108+
assertTrue(transportServiceA.nodeConnected(transportServiceB.getLocalNode()));
109+
}
110+
111+
}

‎server/build.gradle‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -151,30 +151,6 @@ if (buildParams.snapshotBuild == false) {
151151

152152
tasks.named("test").configure {
153153
systemProperty 'es.insecure_network_trace_enabled', 'true'
154-
filter {
155-
excludeTestsMatching("*.TransportServiceHandshakeTests.testAcceptsMismatchedServerlessBuildHash")
156-
}
157-
excludes << '**/IndexSettingsOverrideTests.class'
158-
}
159-
160-
// There are tests rely on system properties to be configured differently. They must run in a separate test job
161-
// since the default does not work for them and configuring the system properties inside the test class/method
162-
// is too late because fields based on the system properties are often initialized statically.
163-
TaskProvider<Test> systemPropertiesOverrideTest = tasks.register("systemPropertiesOverrideTest", Test) {
164-
include '**/IndexSettingsOverrideTests.class'
165-
include '**/TransportServiceHandshakeTests.class'
166-
filter {
167-
includeTestsMatching("*.TransportServiceHandshakeTests.testAcceptsMismatchedServerlessBuildHash")
168-
includeTestsMatching("*.IndexSettingsOverrideTests.*")
169-
}
170-
systemProperty 'es.stateless.allow.index.refresh_interval.override', 'true'
171-
systemProperty 'es.serverless_transport', 'true'
172-
classpath = sourceSets.test.runtimeClasspath
173-
testClassesDirs = sourceSets.test.output.classesDirs
174-
}
175-
176-
tasks.named("check").configure {
177-
dependsOn(systemPropertiesOverrideTest)
178154
}
179155

180156
tasks.named("thirdPartyAudit").configure {

‎server/src/test/java/org/elasticsearch/transport/TransportServiceHandshakeTests.java‎

Lines changed: 4 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
package org.elasticsearch.transport;
1111

1212
import org.apache.logging.log4j.Level;
13-
import org.elasticsearch.Build;
1413
import org.elasticsearch.TransportVersion;
1514
import org.elasticsearch.Version;
1615
import org.elasticsearch.cluster.node.DiscoveryNode;
@@ -41,7 +40,6 @@
4140
import java.util.ArrayList;
4241
import java.util.Collections;
4342
import java.util.List;
44-
import java.util.concurrent.Executor;
4543
import java.util.concurrent.TimeUnit;
4644

4745
import static java.util.Collections.emptySet;
@@ -323,10 +321,8 @@ public void testNodeConnectWithDifferentNodeId() {
323321
}
324322

325323
public void testRejectsMismatchedBuildHash() {
326-
final DisruptingTransportInterceptor transportInterceptorA = new DisruptingTransportInterceptor();
327-
final DisruptingTransportInterceptor transportInterceptorB = new DisruptingTransportInterceptor();
328-
transportInterceptorA.setModifyBuildHash(true);
329-
transportInterceptorB.setModifyBuildHash(true);
324+
final var transportInterceptorA = new BuildHashModifyingTransportInterceptor();
325+
final var transportInterceptorB = new BuildHashModifyingTransportInterceptor();
330326
final Settings settings = Settings.builder()
331327
.put("cluster.name", "a")
332328
.put(IGNORE_DESERIALIZATION_ERRORS_SETTING.getKey(), true) // suppress assertions to test production error-handling
@@ -375,39 +371,9 @@ public void testRejectsMismatchedBuildHash() {
375371
assertFalse(transportServiceA.nodeConnected(discoveryNode));
376372
}
377373

378-
public void testAcceptsMismatchedServerlessBuildHash() {
379-
assumeTrue("Current build needs to be a snapshot", Build.current().isSnapshot());
380-
final DisruptingTransportInterceptor transportInterceptorA = new DisruptingTransportInterceptor();
381-
final DisruptingTransportInterceptor transportInterceptorB = new DisruptingTransportInterceptor();
382-
transportInterceptorA.setModifyBuildHash(true);
383-
transportInterceptorB.setModifyBuildHash(true);
384-
final Settings settings = Settings.builder()
385-
.put("cluster.name", "a")
386-
.put(IGNORE_DESERIALIZATION_ERRORS_SETTING.getKey(), true) // suppress assertions to test production error-handling
387-
.build();
388-
final TransportService transportServiceA = startServices(
389-
"TS_A",
390-
settings,
391-
TransportVersion.current(),
392-
VersionInformation.CURRENT,
393-
transportInterceptorA
394-
);
395-
final TransportService transportServiceB = startServices(
396-
"TS_B",
397-
settings,
398-
TransportVersion.current(),
399-
VersionInformation.CURRENT,
400-
transportInterceptorB
401-
);
402-
AbstractSimpleTransportTestCase.connectToNode(transportServiceA, transportServiceB.getLocalNode(), TestProfiles.LIGHT_PROFILE);
403-
assertTrue(transportServiceA.nodeConnected(transportServiceB.getLocalNode()));
404-
}
405-
406374
public void testAcceptsMismatchedBuildHashFromDifferentVersion() {
407-
final DisruptingTransportInterceptor transportInterceptorA = new DisruptingTransportInterceptor();
408-
final DisruptingTransportInterceptor transportInterceptorB = new DisruptingTransportInterceptor();
409-
transportInterceptorA.setModifyBuildHash(true);
410-
transportInterceptorB.setModifyBuildHash(true);
375+
final var transportInterceptorA = new BuildHashModifyingTransportInterceptor();
376+
final var transportInterceptorB = new BuildHashModifyingTransportInterceptor();
411377
final TransportService transportServiceA = startServices(
412378
"TS_A",
413379
Settings.builder().put("cluster.name", "a").build(),
@@ -425,58 +391,4 @@ public void testAcceptsMismatchedBuildHashFromDifferentVersion() {
425391
AbstractSimpleTransportTestCase.connectToNode(transportServiceA, transportServiceB.getLocalNode(), TestProfiles.LIGHT_PROFILE);
426392
assertTrue(transportServiceA.nodeConnected(transportServiceB.getLocalNode()));
427393
}
428-
429-
private static class DisruptingTransportInterceptor implements TransportInterceptor {
430-
431-
private boolean modifyBuildHash;
432-
433-
public void setModifyBuildHash(boolean modifyBuildHash) {
434-
this.modifyBuildHash = modifyBuildHash;
435-
}
436-
437-
@Override
438-
public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(
439-
String action,
440-
Executor executor,
441-
boolean forceExecution,
442-
TransportRequestHandler<T> actualHandler
443-
) {
444-
445-
if (TransportService.HANDSHAKE_ACTION_NAME.equals(action)) {
446-
return (request, channel, task) -> actualHandler.messageReceived(request, new TransportChannel() {
447-
@Override
448-
public String getProfileName() {
449-
return channel.getProfileName();
450-
}
451-
452-
@Override
453-
public void sendResponse(TransportResponse response) {
454-
assertThat(response, instanceOf(TransportService.HandshakeResponse.class));
455-
if (modifyBuildHash) {
456-
final TransportService.HandshakeResponse handshakeResponse = (TransportService.HandshakeResponse) response;
457-
channel.sendResponse(
458-
new TransportService.HandshakeResponse(
459-
handshakeResponse.getVersion(),
460-
handshakeResponse.getBuildHash() + "-modified",
461-
handshakeResponse.getDiscoveryNode(),
462-
handshakeResponse.getClusterName()
463-
)
464-
);
465-
} else {
466-
channel.sendResponse(response);
467-
}
468-
}
469-
470-
@Override
471-
public void sendResponse(Exception exception) {
472-
channel.sendResponse(exception);
473-
474-
}
475-
}, task);
476-
} else {
477-
return actualHandler;
478-
}
479-
}
480-
}
481-
482394
}

‎test/framework/src/main/java/org/elasticsearch/transport/BuildHashModifyingTransportInterceptor.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.transport;
11+
12+
import org.elasticsearch.test.ESTestCase;
13+
14+
import java.util.concurrent.Executor;
15+
16+
import static org.hamcrest.Matchers.instanceOf;
17+
18+
class BuildHashModifyingTransportInterceptor implements TransportInterceptor {
19+
20+
@Override
21+
public <T extends TransportRequest> TransportRequestHandler<T> interceptHandler(
22+
String action,
23+
Executor executor,
24+
boolean forceExecution,
25+
TransportRequestHandler<T> actualHandler
26+
) {
27+
28+
if (TransportService.HANDSHAKE_ACTION_NAME.equals(action)) {
29+
return (request, channel, task) -> actualHandler.messageReceived(request, new TransportChannel() {
30+
@Override
31+
public String getProfileName() {
32+
return channel.getProfileName();
33+
}
34+
35+
@Override
36+
public void sendResponse(TransportResponse response) {
37+
ESTestCase.assertThat(response, instanceOf(TransportService.HandshakeResponse.class));
38+
final TransportService.HandshakeResponse handshakeResponse = (TransportService.HandshakeResponse) response;
39+
channel.sendResponse(
40+
new TransportService.HandshakeResponse(
41+
handshakeResponse.getVersion(),
42+
handshakeResponse.getBuildHash() + "-modified",
43+
handshakeResponse.getDiscoveryNode(),
44+
handshakeResponse.getClusterName()
45+
)
46+
);
47+
}
48+
49+
@Override
50+
public void sendResponse(Exception exception) {
51+
channel.sendResponse(exception);
52+
53+
}
54+
}, task);
55+
} else {
56+
return actualHandler;
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)