Skip to content

Commit 4c6a9c8

Browse files
breskebybreskeby
committed
Revert "Use stronger typing in SystemPropertyCommandLineArgumentProvider"
This reverts commit 6c8fa63.
1 parent 6c8fa63 commit 4c6a9c8

File tree

5 files changed

+25
-58
lines changed

5 files changed

+25
-58
lines changed

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ 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;
5455

5556
@Inject
5657
public LegacyRestTestBasePlugin(ProviderFactory providerFactory) {
@@ -59,6 +60,7 @@ public LegacyRestTestBasePlugin(ProviderFactory providerFactory) {
5960

6061
@Override
6162
public void apply(Project project) {
63+
this.project = project;
6264
Provider<RestrictedBuildApiService> serviceProvider = project.getGradle()
6365
.getSharedServices()
6466
.registerIfAbsent("restrictedBuildAPI", RestrictedBuildApiService.class, spec -> {
@@ -86,22 +88,13 @@ public void apply(Project project) {
8688
}
8789
SystemPropertyCommandLineArgumentProvider runnerNonInputProperties =
8890
(SystemPropertyCommandLineArgumentProvider) restIntegTestTask.getExtensions().getByName("nonInputProperties");
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-
);
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()));
10295
runnerNonInputProperties.systemProperty(
10396
TESTS_CLUSTER_REMOTE_ACCESS,
104-
providerFactory.provider(() -> String.join(",", cluster.getAllRemoteAccessPortURI()))
97+
() -> String.join(",", cluster.getAllRemoteAccessPortURI())
10598
);
10699
} else {
107100
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: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,11 @@ public void apply(Project project) {
162162

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

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

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

187181
// Register plugins and modules as task inputs and pass paths as system properties to tests
188182
var modulePath = project.getObjects().fileCollection().from(modulesConfiguration);
189-
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_MODULES_PATH_SYSPROP, providerFactory.provider(modulePath::getAsPath));
183+
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_MODULES_PATH_SYSPROP, modulePath::getAsPath);
190184
registerConfigurationInputs(task, modulesConfiguration.getName(), modulePath);
191185
var pluginPath = project.getObjects().fileCollection().from(pluginsConfiguration);
192-
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_PLUGINS_PATH_SYSPROP, providerFactory.provider(pluginPath::getAsPath));
186+
nonInputSystemProperties.systemProperty(TESTS_CLUSTER_PLUGINS_PATH_SYSPROP, pluginPath::getAsPath);
193187
registerConfigurationInputs(
194188
task,
195189
extractedPluginsConfiguration.getName(),
@@ -198,10 +192,7 @@ public void apply(Project project) {
198192

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

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

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

230218
return null;
231219
}

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

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
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;
2221
import org.gradle.api.tasks.SourceSetContainer;
2322
import org.gradle.api.tasks.TaskProvider;
2423
import org.gradle.api.tasks.bundling.Zip;
@@ -51,7 +50,7 @@ public void apply(Project project) {
5150
.getExtensions()
5251
.getByName(TestClustersPlugin.EXTENSION_NAME);
5352
var clusterProvider = testClusters.register(JAVA_REST_TEST);
54-
ProviderFactory providers = project.getProviders();
53+
5554
// Register test task
5655
TaskProvider<StandaloneRestIntegTestTask> javaRestTestTask = project.getTasks()
5756
.register(JAVA_REST_TEST, StandaloneRestIntegTestTask.class, task -> {
@@ -61,19 +60,10 @@ public void apply(Project project) {
6160

6261
var cluster = clusterProvider.get();
6362
var nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
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-
);
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()));
7767
task.getJvmArgumentProviders().add(nonInputProperties);
7868
});
7969

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ 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+
2731
public void systemProperty(String key, Object value) {
2832
systemProperties.put(key, value);
2933
}

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

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
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;
3130
import org.gradle.api.tasks.Copy;
3231
import org.gradle.api.tasks.SourceSet;
3332
import org.gradle.api.tasks.SourceSetContainer;
@@ -119,23 +118,16 @@ private TaskProvider<StandaloneRestIntegTestTask> setupTestTask(
119118
SourceSet testSourceSet,
120119
NamedDomainObjectProvider<ElasticsearchCluster> clusterProvider
121120
) {
122-
ProviderFactory providers = project.getProviders();
123121
return project.getTasks().register(YAML_REST_TEST, StandaloneRestIntegTestTask.class, task -> {
124122
task.useCluster(clusterProvider.get());
125123
task.setTestClassesDirs(testSourceSet.getOutput().getClassesDirs());
126124
task.setClasspath(testSourceSet.getRuntimeClasspath());
127125

128126
var cluster = clusterProvider.get();
129127
var nonInputProperties = new SystemPropertyCommandLineArgumentProvider();
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()));
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());
139131
task.getJvmArgumentProviders().add(nonInputProperties);
140132
task.systemProperty("tests.rest.load_packaged", Boolean.FALSE.toString());
141133
});

0 commit comments

Comments
 (0)