Skip to content

Commit 6c8fa63

Browse files
breskebybreskeby
committed
Use stronger typing in SystemPropertyCommandLineArgumentProvider
1 parent 0f53903 commit 6c8fa63

File tree

5 files changed

+58
-25
lines changed

5 files changed

+58
-25
lines changed

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/LegacyRestTestBasePlugin.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ public class LegacyRestTestBasePlugin implements Plugin<Project> {
5151
private static final String TESTS_CLUSTER_REMOTE_ACCESS = "tests.cluster.remote_access";
5252

5353
private ProviderFactory providerFactory;
54-
private Project project;
5554

5655
@Inject
5756
public LegacyRestTestBasePlugin(ProviderFactory providerFactory) {
@@ -60,7 +59,6 @@ public LegacyRestTestBasePlugin(ProviderFactory providerFactory) {
6059

6160
@Override
6261
public void apply(Project project) {
63-
this.project = project;
6462
Provider<RestrictedBuildApiService> serviceProvider = project.getGradle()
6563
.getSharedServices()
6664
.registerIfAbsent("restrictedBuildAPI", RestrictedBuildApiService.class, spec -> {
@@ -88,13 +86,22 @@ public void apply(Project project) {
8886
}
8987
SystemPropertyCommandLineArgumentProvider runnerNonInputProperties =
9088
(SystemPropertyCommandLineArgumentProvider) restIntegTestTask.getExtensions().getByName("nonInputProperties");
91-
runnerNonInputProperties.systemProperty(TESTS_REST_CLUSTER, () -> String.join(",", cluster.getAllHttpSocketURI()));
92-
runnerNonInputProperties.systemProperty(TESTS_CLUSTER, () -> String.join(",", cluster.getAllTransportPortURI()));
93-
runnerNonInputProperties.systemProperty(TESTS_CLUSTER_NAME, cluster::getName);
94-
runnerNonInputProperties.systemProperty(TESTS_CLUSTER_READINESS, () -> String.join(",", cluster.getAllReadinessPortURI()));
89+
runnerNonInputProperties.systemProperty(
90+
TESTS_REST_CLUSTER,
91+
providerFactory.provider(() -> String.join(",", cluster.getAllHttpSocketURI()))
92+
);
93+
runnerNonInputProperties.systemProperty(
94+
TESTS_CLUSTER,
95+
providerFactory.provider(() -> String.join(",", cluster.getAllTransportPortURI()))
96+
);
97+
runnerNonInputProperties.systemProperty(TESTS_CLUSTER_NAME, providerFactory.provider(cluster::getName));
98+
runnerNonInputProperties.systemProperty(
99+
TESTS_CLUSTER_READINESS,
100+
providerFactory.provider(() -> String.join(",", cluster.getAllReadinessPortURI()))
101+
);
95102
runnerNonInputProperties.systemProperty(
96103
TESTS_CLUSTER_REMOTE_ACCESS,
97-
() -> String.join(",", cluster.getAllRemoteAccessPortURI())
104+
providerFactory.provider(() -> String.join(",", cluster.getAllRemoteAccessPortURI()))
98105
);
99106
} else {
100107
if (systemProperty(TESTS_CLUSTER) == null || systemProperty(TESTS_CLUSTER_NAME) == null) {

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/RestTestBasePlugin.java‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,17 @@ public void apply(Project project) {
162162

163163
// Pass feature metadata on to tests
164164
task.getInputs().files(featureMetadataConfig).withPathSensitivity(PathSensitivity.NONE);
165-
nonInputSystemProperties.systemProperty(TESTS_FEATURES_METADATA_PATH, () -> featureMetadataConfig.getAsPath());
165+
nonInputSystemProperties.systemProperty(
166+
TESTS_FEATURES_METADATA_PATH,
167+
providerFactory.provider(() -> featureMetadataConfig.getAsPath())
168+
);
166169

167170
// Enable parallel execution for these tests since each test gets its own cluster
168171
task.setMaxParallelForks(task.getProject().getGradle().getStartParameter().getMaxWorkerCount() / 2);
169-
nonInputSystemProperties.systemProperty(TESTS_MAX_PARALLEL_FORKS_SYSPROP, () -> String.valueOf(task.getMaxParallelForks()));
172+
nonInputSystemProperties.systemProperty(
173+
TESTS_MAX_PARALLEL_FORKS_SYSPROP,
174+
providerFactory.provider(() -> String.valueOf(task.getMaxParallelForks()))
175+
);
170176

171177
// Disable test failure reporting since this stuff is now captured in build scans
172178
task.getExtensions().getByType(ErrorReportingTestListener.class).setDumpOutputOnFailure(false);
@@ -180,10 +186,10 @@ public void apply(Project project) {
180186

181187
// Register plugins and modules as task inputs and pass paths as system properties to tests
182188
var modulePath = project.getObjects().fileCollection().from(modulesConfiguration);
183-
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_MODULES_PATH_SYSPROP, modulePath::getAsPath);
189+
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_MODULES_PATH_SYSPROP, providerFactory.provider(modulePath::getAsPath));
184190
registerConfigurationInputs(task, modulesConfiguration.getName(), modulePath);
185191
var pluginPath = project.getObjects().fileCollection().from(pluginsConfiguration);
186-
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_PLUGINS_PATH_SYSPROP, pluginPath::getAsPath);
192+
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_PLUGINS_PATH_SYSPROP, providerFactory.provider(pluginPath::getAsPath));
187193
registerConfigurationInputs(
188194
task,
189195
extractedPluginsConfiguration.getName(),
@@ -192,7 +198,10 @@ public void apply(Project project) {
192198

193199
// Wire up integ-test distribution by default for all test tasks
194200
FileCollection extracted = integTestDistro.getExtracted();
195-
nonInputSystemProperties.systemProperty(INTEG_TEST_DISTRIBUTION_SYSPROP, () -> extracted.getSingleFile().getPath());
201+
nonInputSystemProperties.systemProperty(
202+
INTEG_TEST_DISTRIBUTION_SYSPROP,
203+
providerFactory.provider(() -> extracted.getSingleFile().getPath())
204+
);
196205

197206
// Add `usesDefaultDistribution()` extension method to test tasks to indicate they require the default distro
198207
task.getExtensions().getExtraProperties().set("usesDefaultDistribution", new Closure<Void>(task) {
@@ -213,7 +222,10 @@ public Void call(Object... args) {
213222

214223
// If we are using the default distribution we need to register all module feature metadata
215224
task.getInputs().files(defaultDistroFeatureMetadataConfig).withPathSensitivity(PathSensitivity.NONE);
216-
nonInputSystemProperties.systemProperty(TESTS_FEATURES_METADATA_PATH, defaultDistroFeatureMetadataConfig::getAsPath);
225+
nonInputSystemProperties.systemProperty(
226+
TESTS_FEATURES_METADATA_PATH,
227+
providerFactory.provider(defaultDistroFeatureMetadataConfig::getAsPath)
228+
);
217229

218230
return null;
219231
}

‎build-tools/src/main/java/org/elasticsearch/gradle/test/JavaRestTestPlugin.java‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.gradle.api.Plugin;
1919
import org.gradle.api.Project;
2020
import org.gradle.api.plugins.JavaBasePlugin;
21+
import org.gradle.api.provider.ProviderFactory;
2122
import org.gradle.api.tasks.SourceSetContainer;
2223
import org.gradle.api.tasks.TaskProvider;
2324
import org.gradle.api.tasks.bundling.Zip;
@@ -50,7 +51,7 @@ public void apply(Project project) {
5051
.getExtensions()
5152
.getByName(TestClustersPlugin.EXTENSION_NAME);
5253
var clusterProvider = testClusters.register(JAVA_REST_TEST);
53-
54+
ProviderFactory providers = project.getProviders();
5455
// Register test task
5556
TaskProvider<StandaloneRestIntegTestTask> javaRestTestTask = project.getTasks()
5657
.register(JAVA_REST_TEST, StandaloneRestIntegTestTask.class, task -> {
@@ -60,10 +61,19 @@ public void apply(Project project) {
6061

6162
var cluster = clusterProvider.get();
6263
var nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
63-
nonInputProperties.systemProperty("tests.rest.cluster", () -> String.join(",", cluster.getAllHttpSocketURI()));
64-
nonInputProperties.systemProperty("tests.cluster", () -> String.join(",", cluster.getAllTransportPortURI()));
65-
nonInputProperties.systemProperty("tests.clustername", () -> cluster.getName());
66-
nonInputProperties.systemProperty("tests.cluster.readiness", () -> String.join(",", cluster.getAllReadinessPortURI()));
64+
nonInputProperties.systemProperty(
65+
"tests.rest.cluster",
66+
providers.provider(() -> String.join(",", cluster.getAllHttpSocketURI()))
67+
);
68+
nonInputProperties.systemProperty(
69+
"tests.cluster",
70+
providers.provider(() -> String.join(",", cluster.getAllTransportPortURI()))
71+
);
72+
nonInputProperties.systemProperty("tests.clustername", providers.provider(() -> cluster.getName()));
73+
nonInputProperties.systemProperty(
74+
"tests.cluster.readiness",
75+
providers.provider(() -> String.join(",", cluster.getAllReadinessPortURI()))
76+
);
6777
task.getJvmArgumentProviders().add(nonInputProperties);
6878
});
6979

‎build-tools/src/main/java/org/elasticsearch/gradle/test/SystemPropertyCommandLineArgumentProvider.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ public void systemProperty(String key, Provider<Object> value) {
2424
systemProperties.put(key, (Supplier<String>) () -> String.valueOf(value.get()));
2525
}
2626

27-
public void systemProperty(String key, Supplier<String> value) {
28-
systemProperties.put(key, value);
29-
}
30-
3127
public void systemProperty(String key, Object value) {
3228
systemProperties.put(key, value);
3329
}

‎build-tools/src/main/java/org/elasticsearch/gradle/test/YamlRestTestPlugin.java‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.gradle.api.artifacts.type.ArtifactTypeDefinition;
2828
import org.gradle.api.attributes.Attribute;
2929
import org.gradle.api.plugins.JavaBasePlugin;
30+
import org.gradle.api.provider.ProviderFactory;
3031
import org.gradle.api.tasks.Copy;
3132
import org.gradle.api.tasks.SourceSet;
3233
import org.gradle.api.tasks.SourceSetContainer;
@@ -118,16 +119,23 @@ private TaskProvider<StandaloneRestIntegTestTask> setupTestTask(
118119
SourceSet testSourceSet,
119120
NamedDomainObjectProvider<ElasticsearchCluster> clusterProvider
120121
) {
122+
ProviderFactory providers = project.getProviders();
121123
return project.getTasks().register(YAML_REST_TEST, StandaloneRestIntegTestTask.class, task -> {
122124
task.useCluster(clusterProvider.get());
123125
task.setTestClassesDirs(testSourceSet.getOutput().getClassesDirs());
124126
task.setClasspath(testSourceSet.getRuntimeClasspath());
125127

126128
var cluster = clusterProvider.get();
127129
var nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
128-
nonInputProperties.systemProperty("tests.rest.cluster", () -> String.join(",", cluster.getAllHttpSocketURI()));
129-
nonInputProperties.systemProperty("tests.cluster", () -> String.join(",", cluster.getAllTransportPortURI()));
130-
nonInputProperties.systemProperty("tests.clustername", () -> cluster.getName());
130+
nonInputProperties.systemProperty(
131+
"tests.rest.cluster",
132+
providers.provider(() -> String.join(",", cluster.getAllHttpSocketURI()))
133+
);
134+
nonInputProperties.systemProperty(
135+
"tests.cluster",
136+
providers.provider(() -> String.join(",", cluster.getAllTransportPortURI()))
137+
);
138+
nonInputProperties.systemProperty("tests.clustername", providers.provider(() -> cluster.getName()));
131139
task.getJvmArgumentProviders().add(nonInputProperties);
132140
task.systemProperty("tests.rest.load_packaged", Boolean.FALSE.toString());
133141
});

0 commit comments

Comments
 (0)