Skip to content

Commit 8b1affc

Browse files
committed
Remove deprecation warnings to prepare for Gradle 5
Gradle replaced `project.sourceSets.main.output.classesDir` of type `File` with `project.sourceSets.main.output.classesDirs` of type `FileCollection` (see [SourceSetOutput](https://github.com/gradle/gradle/blob/master/subprojects/plugins/src/main/java/org/gradle/api/tasks/SourceSetOutput.java)) Build output is now stored on a per language folder. There are a few places where we use that, here's these and how it's fixed: - Randomized Test execution - look in all test folders ( pass the multi dir configuration to the ant runner ) - DRY the task configuration by introducing `basedOn` for `RandomizedTestingTask` DSL - Extend the naming convention test to support passing in multiple directories - Fix the standalon test plugin, the dires were not passed trough, checked with a debuger and the statement had no affect due to a missing `=`. Closes #30354
1 parent 65dbc17 commit 8b1affc

File tree

13 files changed

+83
-93
lines changed

13 files changed

+83
-93
lines changed

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class RandomizedTestingPlugin implements Plugin<Project> {
1313

1414
void apply(Project project) {
1515
setupSeed(project)
16-
replaceTestTask(project.tasks)
16+
replaceTestTask(project)
1717
configureAnt(project.ant)
1818
}
1919

@@ -44,24 +44,24 @@ class RandomizedTestingPlugin implements Plugin<Project> {
4444
}
4545
}
4646

47-
static void replaceTestTask(TaskContainer tasks) {
47+
static void replaceTestTask(Project project) {
48+
def tasks = project.tasks
4849
Test oldTestTask = tasks.findByPath('test')
4950
if (oldTestTask == null) {
5051
// no test task, ok, user will use testing task on their own
5152
return
5253
}
5354
tasks.remove(oldTestTask)
5455

55-
Map properties = [
56+
RandomizedTestingTask newTestTask = tasks.create([
5657
name: 'test',
5758
type: RandomizedTestingTask,
5859
dependsOn: oldTestTask.dependsOn,
5960
group: JavaBasePlugin.VERIFICATION_GROUP,
6061
description: 'Runs unit tests with the randomized testing framework'
61-
]
62-
RandomizedTestingTask newTestTask = tasks.create(properties)
62+
])
6363
newTestTask.classpath = oldTestTask.classpath
64-
newTestTask.testClassesDir = oldTestTask.project.sourceSets.test.output.classesDir
64+
newTestTask.testClassesDirs = oldTestTask.project.sourceSets.test.output.classesDirs
6565
// since gradle 4.5, tasks immutable dependencies are "hidden" (do not show up in dependsOn)
6666
// so we must explicitly add a dependency on generating the test classpath
6767
newTestTask.dependsOn('testClasses')

buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,20 @@ import groovy.xml.NamespaceBuilder
66
import groovy.xml.NamespaceBuilderSupport
77
import org.apache.tools.ant.BuildException
88
import org.apache.tools.ant.DefaultLogger
9+
import org.apache.tools.ant.Project
910
import org.apache.tools.ant.RuntimeConfigurable
1011
import org.apache.tools.ant.UnknownElement
12+
import org.elasticsearch.gradle.BuildPlugin
1113
import org.gradle.api.DefaultTask
1214
import org.gradle.api.InvalidUserDataException
1315
import org.gradle.api.file.FileCollection
1416
import org.gradle.api.file.FileTreeElement
15-
import org.gradle.api.internal.tasks.options.Option
1617
import org.gradle.api.specs.Spec
1718
import org.gradle.api.tasks.Input
1819
import org.gradle.api.tasks.InputDirectory
1920
import org.gradle.api.tasks.Optional
2021
import org.gradle.api.tasks.TaskAction
22+
import org.gradle.api.tasks.options.Option
2123
import org.gradle.api.tasks.util.PatternFilterable
2224
import org.gradle.api.tasks.util.PatternSet
2325
import org.gradle.internal.logging.progress.ProgressLoggerFactory
@@ -43,8 +45,8 @@ class RandomizedTestingTask extends DefaultTask {
4345
@Input
4446
String parallelism = '1'
4547

46-
@InputDirectory
47-
File testClassesDir
48+
@Input
49+
FileCollection testClassesDirs
4850

4951
@Optional
5052
@Input
@@ -95,6 +97,17 @@ class RandomizedTestingTask extends DefaultTask {
9597
listenersConfig.listeners.add(new TestReportLogger(logger: logger, config: testLoggingConfig))
9698
}
9799

100+
void basedOn(RandomizedTestingTask other) {
101+
configure(BuildPlugin.commonTestConfig(project))
102+
classpath = other.classpath;
103+
testClassesDirs = other.testClassesDirs;
104+
dependsOn = other.dependsOn
105+
other.mustRunAfter this
106+
if (project.tasks.findByPath("check") != null) {
107+
project.tasks.check.dependsOn this
108+
}
109+
}
110+
98111
@Inject
99112
ProgressLoggerFactory getProgressLoggerFactory() {
100113
throw new UnsupportedOperationException()
@@ -211,7 +224,7 @@ class RandomizedTestingTask extends DefaultTask {
211224

212225
DefaultLogger listener = null
213226
ByteArrayOutputStream antLoggingBuffer = null
214-
if (logger.isInfoEnabled() == false) {
227+
if (!logger.isInfoEnabled()) {
215228
// in info logging, ant already outputs info level, so we see everything
216229
// but on errors or when debugging, we want to see info level messages
217230
// because junit4 emits jvm output with ant logging
@@ -220,15 +233,15 @@ class RandomizedTestingTask extends DefaultTask {
220233
listener = new DefaultLogger(
221234
errorPrintStream: System.err,
222235
outputPrintStream: System.out,
223-
messageOutputLevel: org.apache.tools.ant.Project.MSG_INFO)
236+
messageOutputLevel: Project.MSG_INFO)
224237
} else {
225238
// we want to buffer the info, and emit it if the test fails
226239
antLoggingBuffer = new ByteArrayOutputStream()
227240
PrintStream stream = new PrintStream(antLoggingBuffer, true, "UTF-8")
228241
listener = new DefaultLogger(
229242
errorPrintStream: stream,
230243
outputPrintStream: stream,
231-
messageOutputLevel: org.apache.tools.ant.Project.MSG_INFO)
244+
messageOutputLevel: Project.MSG_INFO)
232245
}
233246
project.ant.project.addBuildListener(listener)
234247
}
@@ -251,12 +264,10 @@ class RandomizedTestingTask extends DefaultTask {
251264
if (argLine != null) {
252265
jvmarg(line: argLine)
253266
}
254-
fileset(dir: testClassesDir) {
255-
for (String includePattern : patternSet.getIncludes()) {
256-
include(name: includePattern)
257-
}
258-
for (String excludePattern : patternSet.getExcludes()) {
259-
exclude(name: excludePattern)
267+
testClassesDirs.each { testClassDir ->
268+
fileset(dir: testClassDir) {
269+
patternSet.getIncludes().each { include(name: it) }
270+
patternSet.getExcludes().each { exclude(name: it) }
260271
}
261272
}
262273
for (Map.Entry<String, Object> prop : systemProperties) {

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -726,7 +726,7 @@ class BuildPlugin implements Plugin<Project> {
726726
project.extensions.add('additionalTest', { String name, Closure config ->
727727
RandomizedTestingTask additionalTest = project.tasks.create(name, RandomizedTestingTask.class)
728728
additionalTest.classpath = test.classpath
729-
additionalTest.testClassesDir = test.testClassesDir
729+
additionalTest.testClassesDirs = test.testClassesDirs
730730
additionalTest.configure(commonTestConfig(project))
731731
additionalTest.configure(config)
732732
test.dependsOn(additionalTest)

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ public class LoggerUsageTask extends LoggedExec {
5050
List files = []
5151
// But only if the source sets that will make them exist
5252
if (project.sourceSets.findByName("main")) {
53-
files.add(project.sourceSets.main.output.classesDir)
53+
files.add(project.sourceSets.main.output.classesDirs.getFiles())
5454
dependsOn project.tasks.classes
5555
}
5656
if (project.sourceSets.findByName("test")) {
57-
files.add(project.sourceSets.test.output.classesDir)
57+
files.add(project.sourceSets.test.output.classesDirs.getFiles())
5858
dependsOn project.tasks.testClasses
5959
}
6060
/* In an extra twist, it isn't good enough that the source set

buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import org.elasticsearch.gradle.VersionProperties
2424
import org.gradle.api.artifacts.Dependency
2525
import org.gradle.api.file.FileCollection
2626
import org.gradle.api.tasks.Input
27-
import org.gradle.api.tasks.InputFiles
2827
import org.gradle.api.tasks.OutputFile
2928
/**
3029
* Runs NamingConventionsCheck on a classpath/directory combo to verify that
@@ -66,9 +65,9 @@ public class NamingConventionsTask extends LoggedExec {
6665
@Input
6766
boolean checkForTestsInMain = false;
6867

69-
public NamingConventionsTask() {
68+
NamingConventionsTask() {
7069
// Extra classpath contains the actual test
71-
if (false == project.configurations.names.contains('namingConventions')) {
70+
if (!project.configurations.names.contains('namingConventions')) {
7271
project.configurations.create('namingConventions')
7372
Dependency buildToolsDep = project.dependencies.add('namingConventions',
7473
"org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}")
@@ -81,11 +80,18 @@ public class NamingConventionsTask extends LoggedExec {
8180
inputs.files(classpath)
8281
description = "Tests that test classes aren't misnamed or misplaced"
8382
executable = new File(project.runtimeJavaHome, 'bin/java')
84-
if (false == checkForTestsInMain) {
83+
84+
def dirsToUse = (checkForTestsInMain ?
85+
project.sourceSets.main.output.classesDirs :
86+
project.sourceSets.test.output.classesDirs)
87+
.filter {it.exists()}
88+
.collect {it.getAbsolutePath()}
89+
90+
if (!checkForTestsInMain) {
8591
/* This task is created by default for all subprojects with this
8692
* setting and there is no point in running it if the files don't
8793
* exist. */
88-
onlyIf { project.sourceSets.test.output.classesDir.exists() }
94+
onlyIf { dirsToUse.size() != 0 }
8995
}
9096

9197
/*
@@ -111,16 +117,17 @@ public class NamingConventionsTask extends LoggedExec {
111117
if (':build-tools'.equals(project.path)) {
112118
args('--self-test')
113119
}
120+
114121
if (checkForTestsInMain) {
115122
args('--main')
116123
args('--')
117-
args(project.sourceSets.main.output.classesDir.absolutePath)
118124
} else {
119125
args('--')
120-
args(project.sourceSets.test.output.classesDir.absolutePath)
121126
}
127+
args(dirsToUse.join(File.pathSeparator))
122128
}
123129
}
124130
doLast { successMarker.setText("", 'UTF-8') }
125131
}
132+
126133
}

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public class RestIntegTestTask extends DefaultTask {
6161
clusterInit = project.tasks.create(name: "${name}Cluster#init", dependsOn: project.testClasses)
6262
runner.dependsOn(clusterInit)
6363
runner.classpath = project.sourceSets.test.runtimeClasspath
64-
runner.testClassesDir = project.sourceSets.test.output.classesDir
64+
runner.testClassesDirs = project.sourceSets.test.output.classesDirs
6565
clusterConfig = project.extensions.create("${name}Cluster", ClusterConfiguration.class, project)
6666

6767
// start with the common test configuration

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneTestPlugin.groovy

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class StandaloneTestPlugin implements Plugin<Project> {
4646
test.configure(BuildPlugin.commonTestConfig(project))
4747
BuildPlugin.configureCompile(project)
4848
test.classpath = project.sourceSets.test.runtimeClasspath
49-
test.testClassesDir project.sourceSets.test.output.classesDir
49+
test.testClassesDirs = project.sourceSets.test.output.classesDirs
5050
test.mustRunAfter(project.precommit)
5151
project.check.dependsOn(test)
5252
}

buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
package org.elasticsearch.test;
2121

22+
import java.io.File;
2223
import java.io.IOException;
2324
import java.lang.reflect.Modifier;
2425
import java.nio.file.FileVisitResult;
@@ -30,6 +31,7 @@
3031
import java.util.HashSet;
3132
import java.util.Objects;
3233
import java.util.Set;
34+
import java.util.regex.Pattern;
3335

3436
/**
3537
* Checks that all tests in a directory are named according to our naming conventions. This is important because tests that do not follow
@@ -47,7 +49,7 @@ public class NamingConventionsCheck {
4749
public static void main(String[] args) throws IOException {
4850
Class<?> testClass = null;
4951
Class<?> integTestClass = null;
50-
Path rootPath = null;
52+
String rootPathList = null;
5153
boolean skipIntegTestsInDisguise = false;
5254
boolean selfTest = false;
5355
boolean checkMainClasses = false;
@@ -70,18 +72,21 @@ public static void main(String[] args) throws IOException {
7072
checkMainClasses = true;
7173
break;
7274
case "--":
73-
rootPath = Paths.get(args[++i]);
75+
rootPathList = args[++i];
7476
break;
7577
default:
7678
fail("unsupported argument '" + arg + "'");
7779
}
7880
}
7981

8082
NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass);
81-
if (checkMainClasses) {
82-
check.checkMain(rootPath);
83-
} else {
84-
check.checkTests(rootPath, skipIntegTestsInDisguise);
83+
for (String rootDir : rootPathList.split(Pattern.quote(File.pathSeparator))) {
84+
Path rootPath = Paths.get(rootDir);
85+
if (checkMainClasses) {
86+
check.checkMain(rootPath);
87+
} else {
88+
check.checkTests(rootPath, skipIntegTestsInDisguise);
89+
}
8590
}
8691

8792
if (selfTest) {

server/build.gradle

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -326,15 +326,9 @@ dependencyLicenses {
326326
}
327327

328328
if (isEclipse == false || project.path == ":server-tests") {
329-
task integTest(type: RandomizedTestingTask,
330-
group: JavaBasePlugin.VERIFICATION_GROUP,
331-
description: 'Multi-node tests',
332-
dependsOn: test.dependsOn) {
333-
configure(BuildPlugin.commonTestConfig(project))
334-
classpath = project.test.classpath
335-
testClassesDir = project.test.testClassesDir
329+
task integTest(type: RandomizedTestingTask,group: JavaBasePlugin.VERIFICATION_GROUP) {
330+
description = 'Multi-node tests'
331+
basedOn tasks.test
336332
include '**/*IT.class'
337333
}
338-
check.dependsOn integTest
339-
integTest.mustRunAfter test
340334
}

x-pack/plugin/ml/build.gradle

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,13 @@ run {
9090
// so we disable integ tests
9191
integTest.enabled = false
9292

93-
// Instead we create a separate task to run the
94-
// tests based on ESIntegTestCase
95-
task internalClusterTest(type: RandomizedTestingTask,
96-
group: JavaBasePlugin.VERIFICATION_GROUP,
97-
description: 'Multi-node tests',
98-
dependsOn: test.dependsOn) {
99-
configure(BuildPlugin.commonTestConfig(project))
100-
classpath = project.test.classpath
101-
testClassesDir = project.test.testClassesDir
102-
include '**/*IT.class'
103-
systemProperty 'es.set.netty.runtime.available.processors', 'false'
93+
// Instead we create a separate task to run the tests based on ESIntegTestCase
94+
task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) {
95+
description = 'Multi-node tests'
96+
basedOn tasks.test
97+
include '**/*IT.class'
98+
systemProperty 'es.set.netty.runtime.available.processors', 'false'
10499
}
105-
check.dependsOn internalClusterTest
106-
internalClusterTest.mustRunAfter test
107100

108101
// also add an "alias" task to make typing on the command line easier
109102
task icTest {

0 commit comments

Comments
 (0)