From 8b1affcc98cab718834ee6a1b11bd62b1f8d3507 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 3 May 2018 14:32:09 +0300 Subject: [PATCH 01/19] 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 --- .../junit4/RandomizedTestingPlugin.groovy | 12 +++---- .../junit4/RandomizedTestingTask.groovy | 35 ++++++++++++------- .../elasticsearch/gradle/BuildPlugin.groovy | 2 +- .../gradle/precommit/LoggerUsageTask.groovy | 4 +-- .../precommit/NamingConventionsTask.groovy | 21 +++++++---- .../gradle/test/RestIntegTestTask.groovy | 2 +- .../gradle/test/StandaloneTestPlugin.groovy | 2 +- .../test/NamingConventionsCheck.java | 17 +++++---- server/build.gradle | 12 ++----- x-pack/plugin/ml/build.gradle | 19 ++++------ x-pack/plugin/monitoring/build.gradle | 19 ++++------ x-pack/plugin/rollup/build.gradle | 15 +++----- x-pack/plugin/upgrade/build.gradle | 16 +++------ 13 files changed, 83 insertions(+), 93 deletions(-) diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy index c375f773bf9b5..3a805b9902ab9 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy @@ -13,7 +13,7 @@ class RandomizedTestingPlugin implements Plugin { void apply(Project project) { setupSeed(project) - replaceTestTask(project.tasks) + replaceTestTask(project) configureAnt(project.ant) } @@ -44,7 +44,8 @@ class RandomizedTestingPlugin implements Plugin { } } - static void replaceTestTask(TaskContainer tasks) { + static void replaceTestTask(Project project) { + def tasks = project.tasks Test oldTestTask = tasks.findByPath('test') if (oldTestTask == null) { // no test task, ok, user will use testing task on their own @@ -52,16 +53,15 @@ class RandomizedTestingPlugin implements Plugin { } tasks.remove(oldTestTask) - Map properties = [ + RandomizedTestingTask newTestTask = tasks.create([ name: 'test', type: RandomizedTestingTask, dependsOn: oldTestTask.dependsOn, group: JavaBasePlugin.VERIFICATION_GROUP, description: 'Runs unit tests with the randomized testing framework' - ] - RandomizedTestingTask newTestTask = tasks.create(properties) + ]) newTestTask.classpath = oldTestTask.classpath - newTestTask.testClassesDir = oldTestTask.project.sourceSets.test.output.classesDir + newTestTask.testClassesDirs = oldTestTask.project.sourceSets.test.output.classesDirs // since gradle 4.5, tasks immutable dependencies are "hidden" (do not show up in dependsOn) // so we must explicitly add a dependency on generating the test classpath newTestTask.dependsOn('testClasses') diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy index 1817ea57e7abe..9902ae7b1023b 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy @@ -6,18 +6,20 @@ import groovy.xml.NamespaceBuilder import groovy.xml.NamespaceBuilderSupport import org.apache.tools.ant.BuildException import org.apache.tools.ant.DefaultLogger +import org.apache.tools.ant.Project import org.apache.tools.ant.RuntimeConfigurable import org.apache.tools.ant.UnknownElement +import org.elasticsearch.gradle.BuildPlugin import org.gradle.api.DefaultTask import org.gradle.api.InvalidUserDataException import org.gradle.api.file.FileCollection import org.gradle.api.file.FileTreeElement -import org.gradle.api.internal.tasks.options.Option import org.gradle.api.specs.Spec import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputDirectory import org.gradle.api.tasks.Optional import org.gradle.api.tasks.TaskAction +import org.gradle.api.tasks.options.Option import org.gradle.api.tasks.util.PatternFilterable import org.gradle.api.tasks.util.PatternSet import org.gradle.internal.logging.progress.ProgressLoggerFactory @@ -43,8 +45,8 @@ class RandomizedTestingTask extends DefaultTask { @Input String parallelism = '1' - @InputDirectory - File testClassesDir + @Input + FileCollection testClassesDirs @Optional @Input @@ -95,6 +97,17 @@ class RandomizedTestingTask extends DefaultTask { listenersConfig.listeners.add(new TestReportLogger(logger: logger, config: testLoggingConfig)) } + void basedOn(RandomizedTestingTask other) { + configure(BuildPlugin.commonTestConfig(project)) + classpath = other.classpath; + testClassesDirs = other.testClassesDirs; + dependsOn = other.dependsOn + other.mustRunAfter this + if (project.tasks.findByPath("check") != null) { + project.tasks.check.dependsOn this + } + } + @Inject ProgressLoggerFactory getProgressLoggerFactory() { throw new UnsupportedOperationException() @@ -211,7 +224,7 @@ class RandomizedTestingTask extends DefaultTask { DefaultLogger listener = null ByteArrayOutputStream antLoggingBuffer = null - if (logger.isInfoEnabled() == false) { + if (!logger.isInfoEnabled()) { // in info logging, ant already outputs info level, so we see everything // but on errors or when debugging, we want to see info level messages // because junit4 emits jvm output with ant logging @@ -220,7 +233,7 @@ class RandomizedTestingTask extends DefaultTask { listener = new DefaultLogger( errorPrintStream: System.err, outputPrintStream: System.out, - messageOutputLevel: org.apache.tools.ant.Project.MSG_INFO) + messageOutputLevel: Project.MSG_INFO) } else { // we want to buffer the info, and emit it if the test fails antLoggingBuffer = new ByteArrayOutputStream() @@ -228,7 +241,7 @@ class RandomizedTestingTask extends DefaultTask { listener = new DefaultLogger( errorPrintStream: stream, outputPrintStream: stream, - messageOutputLevel: org.apache.tools.ant.Project.MSG_INFO) + messageOutputLevel: Project.MSG_INFO) } project.ant.project.addBuildListener(listener) } @@ -251,12 +264,10 @@ class RandomizedTestingTask extends DefaultTask { if (argLine != null) { jvmarg(line: argLine) } - fileset(dir: testClassesDir) { - for (String includePattern : patternSet.getIncludes()) { - include(name: includePattern) - } - for (String excludePattern : patternSet.getExcludes()) { - exclude(name: excludePattern) + testClassesDirs.each { testClassDir -> + fileset(dir: testClassDir) { + patternSet.getIncludes().each { include(name: it) } + patternSet.getExcludes().each { exclude(name: it) } } } for (Map.Entry prop : systemProperties) { diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index a44b9c849d333..f1f303865959b 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -726,7 +726,7 @@ class BuildPlugin implements Plugin { project.extensions.add('additionalTest', { String name, Closure config -> RandomizedTestingTask additionalTest = project.tasks.create(name, RandomizedTestingTask.class) additionalTest.classpath = test.classpath - additionalTest.testClassesDir = test.testClassesDir + additionalTest.testClassesDirs = test.testClassesDirs additionalTest.configure(commonTestConfig(project)) additionalTest.configure(config) test.dependsOn(additionalTest) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy index 87b73795604ab..8e16b83cebc0b 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy @@ -50,11 +50,11 @@ public class LoggerUsageTask extends LoggedExec { List files = [] // But only if the source sets that will make them exist if (project.sourceSets.findByName("main")) { - files.add(project.sourceSets.main.output.classesDir) + files.add(project.sourceSets.main.output.classesDirs.getFiles()) dependsOn project.tasks.classes } if (project.sourceSets.findByName("test")) { - files.add(project.sourceSets.test.output.classesDir) + files.add(project.sourceSets.test.output.classesDirs.getFiles()) dependsOn project.tasks.testClasses } /* In an extra twist, it isn't good enough that the source set diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy index 6050d4e278dd6..9450d62817c0e 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy @@ -24,7 +24,6 @@ import org.elasticsearch.gradle.VersionProperties import org.gradle.api.artifacts.Dependency import org.gradle.api.file.FileCollection import org.gradle.api.tasks.Input -import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.OutputFile /** * Runs NamingConventionsCheck on a classpath/directory combo to verify that @@ -66,9 +65,9 @@ public class NamingConventionsTask extends LoggedExec { @Input boolean checkForTestsInMain = false; - public NamingConventionsTask() { + NamingConventionsTask() { // Extra classpath contains the actual test - if (false == project.configurations.names.contains('namingConventions')) { + if (!project.configurations.names.contains('namingConventions')) { project.configurations.create('namingConventions') Dependency buildToolsDep = project.dependencies.add('namingConventions', "org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}") @@ -81,11 +80,18 @@ public class NamingConventionsTask extends LoggedExec { inputs.files(classpath) description = "Tests that test classes aren't misnamed or misplaced" executable = new File(project.runtimeJavaHome, 'bin/java') - if (false == checkForTestsInMain) { + + def dirsToUse = (checkForTestsInMain ? + project.sourceSets.main.output.classesDirs : + project.sourceSets.test.output.classesDirs) + .filter {it.exists()} + .collect {it.getAbsolutePath()} + + if (!checkForTestsInMain) { /* This task is created by default for all subprojects with this * setting and there is no point in running it if the files don't * exist. */ - onlyIf { project.sourceSets.test.output.classesDir.exists() } + onlyIf { dirsToUse.size() != 0 } } /* @@ -111,16 +117,17 @@ public class NamingConventionsTask extends LoggedExec { if (':build-tools'.equals(project.path)) { args('--self-test') } + if (checkForTestsInMain) { args('--main') args('--') - args(project.sourceSets.main.output.classesDir.absolutePath) } else { args('--') - args(project.sourceSets.test.output.classesDir.absolutePath) } + args(dirsToUse.join(File.pathSeparator)) } } doLast { successMarker.setText("", 'UTF-8') } } + } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy index 242ed45eee86e..bcd9321782c02 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/RestIntegTestTask.groovy @@ -61,7 +61,7 @@ public class RestIntegTestTask extends DefaultTask { clusterInit = project.tasks.create(name: "${name}Cluster#init", dependsOn: project.testClasses) runner.dependsOn(clusterInit) runner.classpath = project.sourceSets.test.runtimeClasspath - runner.testClassesDir = project.sourceSets.test.output.classesDir + runner.testClassesDirs = project.sourceSets.test.output.classesDirs clusterConfig = project.extensions.create("${name}Cluster", ClusterConfiguration.class, project) // start with the common test configuration diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneTestPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneTestPlugin.groovy index de52d75c6008c..13f716d831941 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneTestPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneTestPlugin.groovy @@ -46,7 +46,7 @@ public class StandaloneTestPlugin implements Plugin { test.configure(BuildPlugin.commonTestConfig(project)) BuildPlugin.configureCompile(project) test.classpath = project.sourceSets.test.runtimeClasspath - test.testClassesDir project.sourceSets.test.output.classesDir + test.testClassesDirs = project.sourceSets.test.output.classesDirs test.mustRunAfter(project.precommit) project.check.dependsOn(test) } diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java index 9bd14675d34a4..bc5c1fb1b1ffc 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java +++ b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java @@ -19,6 +19,7 @@ package org.elasticsearch.test; +import java.io.File; import java.io.IOException; import java.lang.reflect.Modifier; import java.nio.file.FileVisitResult; @@ -30,6 +31,7 @@ import java.util.HashSet; import java.util.Objects; import java.util.Set; +import java.util.regex.Pattern; /** * 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 { public static void main(String[] args) throws IOException { Class testClass = null; Class integTestClass = null; - Path rootPath = null; + String rootPathList = null; boolean skipIntegTestsInDisguise = false; boolean selfTest = false; boolean checkMainClasses = false; @@ -70,7 +72,7 @@ public static void main(String[] args) throws IOException { checkMainClasses = true; break; case "--": - rootPath = Paths.get(args[++i]); + rootPathList = args[++i]; break; default: fail("unsupported argument '" + arg + "'"); @@ -78,10 +80,13 @@ public static void main(String[] args) throws IOException { } NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass); - if (checkMainClasses) { - check.checkMain(rootPath); - } else { - check.checkTests(rootPath, skipIntegTestsInDisguise); + for (String rootDir : rootPathList.split(Pattern.quote(File.pathSeparator))) { + Path rootPath = Paths.get(rootDir); + if (checkMainClasses) { + check.checkMain(rootPath); + } else { + check.checkTests(rootPath, skipIntegTestsInDisguise); + } } if (selfTest) { diff --git a/server/build.gradle b/server/build.gradle index 7e880e0dae4d2..f9e3dad1be60a 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -326,15 +326,9 @@ dependencyLicenses { } if (isEclipse == false || project.path == ":server-tests") { - task integTest(type: RandomizedTestingTask, - group: JavaBasePlugin.VERIFICATION_GROUP, - description: 'Multi-node tests', - dependsOn: test.dependsOn) { - configure(BuildPlugin.commonTestConfig(project)) - classpath = project.test.classpath - testClassesDir = project.test.testClassesDir + task integTest(type: RandomizedTestingTask,group: JavaBasePlugin.VERIFICATION_GROUP) { + description = 'Multi-node tests' + basedOn tasks.test include '**/*IT.class' } - check.dependsOn integTest - integTest.mustRunAfter test } diff --git a/x-pack/plugin/ml/build.gradle b/x-pack/plugin/ml/build.gradle index d9d4882b00e1c..116906d08b937 100644 --- a/x-pack/plugin/ml/build.gradle +++ b/x-pack/plugin/ml/build.gradle @@ -90,20 +90,13 @@ run { // so we disable integ tests integTest.enabled = false -// Instead we create a separate task to run the -// tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, - group: JavaBasePlugin.VERIFICATION_GROUP, - description: 'Multi-node tests', - dependsOn: test.dependsOn) { - configure(BuildPlugin.commonTestConfig(project)) - classpath = project.test.classpath - testClassesDir = project.test.testClassesDir - include '**/*IT.class' - systemProperty 'es.set.netty.runtime.available.processors', 'false' +// Instead we create a separate task to run the tests based on ESIntegTestCase +task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { + description = 'Multi-node tests' + basedOn tasks.test + include '**/*IT.class' + systemProperty 'es.set.netty.runtime.available.processors', 'false' } -check.dependsOn internalClusterTest -internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { diff --git a/x-pack/plugin/monitoring/build.gradle b/x-pack/plugin/monitoring/build.gradle index fbdb388e78e19..ae273e33d8335 100644 --- a/x-pack/plugin/monitoring/build.gradle +++ b/x-pack/plugin/monitoring/build.gradle @@ -54,20 +54,13 @@ run { // so we disable integ tests integTest.enabled = false -// Instead we create a separate task to run the -// tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, - group: JavaBasePlugin.VERIFICATION_GROUP, - description: 'Multi-node tests', - dependsOn: test.dependsOn) { - configure(BuildPlugin.commonTestConfig(project)) - classpath = project.test.classpath - testClassesDir = project.test.testClassesDir - include '**/*IT.class' - systemProperty 'es.set.netty.runtime.available.processors', 'false' +// Instead we create a separate task to run the tests based on ESIntegTestCase +task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { + description = 'Multi-node tests' + basedOn test + include '**/*IT.class' + systemProperty 'es.set.netty.runtime.available.processors', 'false' } -check.dependsOn internalClusterTest -internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { task icTest { diff --git a/x-pack/plugin/rollup/build.gradle b/x-pack/plugin/rollup/build.gradle index d8ce1ca304763..7628d806ed273 100644 --- a/x-pack/plugin/rollup/build.gradle +++ b/x-pack/plugin/rollup/build.gradle @@ -34,20 +34,13 @@ run { integTest.enabled = false -// Instead we create a separate task to run the -// tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, - group: JavaBasePlugin.VERIFICATION_GROUP, - description: 'Multi-node tests', - dependsOn: test.dependsOn) { - configure(BuildPlugin.commonTestConfig(project)) - classpath = project.test.classpath - testClassesDir = project.test.testClassesDir +// Instead we create a separate task to run the tests based on ESIntegTestCase +task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { + description = 'Multi-node tests' + basedOn tasks.test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' } -check.dependsOn internalClusterTest -internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { task icTest { diff --git a/x-pack/plugin/upgrade/build.gradle b/x-pack/plugin/upgrade/build.gradle index 5cead96ac7aa5..487d83c37ed94 100644 --- a/x-pack/plugin/upgrade/build.gradle +++ b/x-pack/plugin/upgrade/build.gradle @@ -29,18 +29,12 @@ integTest.enabled = false // Instead we create a separate task to run the // tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, - group: JavaBasePlugin.VERIFICATION_GROUP, - description: 'Multi-node tests', - dependsOn: test.dependsOn) { - configure(BuildPlugin.commonTestConfig(project)) - classpath = project.test.classpath - testClassesDir = project.test.testClassesDir - include '**/*IT.class' - systemProperty 'es.set.netty.runtime.available.processors', 'false' +task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { + description = 'Multi-node tests' + basedOn tasks.test + include '**/*IT.class' + systemProperty 'es.set.netty.runtime.available.processors', 'false' } -check.dependsOn internalClusterTest -internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { From 9bd9389875d8b88aadb50df57a45cd0d2b073241 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 7 May 2018 18:53:04 +0300 Subject: [PATCH 02/19] Only check Java tests, PR feedback - Name checker was ran for Groovy tests that don't adhere to the same convections causing the check to fail - implement PR feedback --- .../junit4/RandomizedTestingPlugin.groovy | 2 +- .../junit4/RandomizedTestingTask.groovy | 3 +-- .../precommit/NamingConventionsTask.groovy | 24 +++++++++++-------- server/build.gradle | 1 + x-pack/plugin/ml/build.gradle | 1 + x-pack/plugin/monitoring/build.gradle | 1 + x-pack/plugin/rollup/build.gradle | 1 + x-pack/plugin/upgrade/build.gradle | 1 + 8 files changed, 21 insertions(+), 13 deletions(-) diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy index 3a805b9902ab9..06f7204027bc6 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy @@ -45,7 +45,7 @@ class RandomizedTestingPlugin implements Plugin { } static void replaceTestTask(Project project) { - def tasks = project.tasks + TaskContainer tasks = project.tasks Test oldTestTask = tasks.findByPath('test') if (oldTestTask == null) { // no test task, ok, user will use testing task on their own diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy index 9902ae7b1023b..954aed43f7260 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy @@ -98,7 +98,6 @@ class RandomizedTestingTask extends DefaultTask { } void basedOn(RandomizedTestingTask other) { - configure(BuildPlugin.commonTestConfig(project)) classpath = other.classpath; testClassesDirs = other.testClassesDirs; dependsOn = other.dependsOn @@ -224,7 +223,7 @@ class RandomizedTestingTask extends DefaultTask { DefaultLogger listener = null ByteArrayOutputStream antLoggingBuffer = null - if (!logger.isInfoEnabled()) { + if (false == logger.isInfoEnabled()) { // in info logging, ant already outputs info level, so we see everything // but on errors or when debugging, we want to see info level messages // because junit4 emits jvm output with ant logging diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy index 9450d62817c0e..ecad31db242c4 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy @@ -67,7 +67,7 @@ public class NamingConventionsTask extends LoggedExec { NamingConventionsTask() { // Extra classpath contains the actual test - if (!project.configurations.names.contains('namingConventions')) { + if (false == project.configurations.names.contains('namingConventions')) { project.configurations.create('namingConventions') Dependency buildToolsDep = project.dependencies.add('namingConventions', "org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}") @@ -81,17 +81,11 @@ public class NamingConventionsTask extends LoggedExec { description = "Tests that test classes aren't misnamed or misplaced" executable = new File(project.runtimeJavaHome, 'bin/java') - def dirsToUse = (checkForTestsInMain ? - project.sourceSets.main.output.classesDirs : - project.sourceSets.test.output.classesDirs) - .filter {it.exists()} - .collect {it.getAbsolutePath()} - - if (!checkForTestsInMain) { + if (false == checkForTestsInMain) { /* This task is created by default for all subprojects with this * setting and there is no point in running it if the files don't * exist. */ - onlyIf { dirsToUse.size() != 0 } + onlyIf { getJavaClassesDir() != null && getJavaClassesDir().exists() } } /* @@ -124,10 +118,20 @@ public class NamingConventionsTask extends LoggedExec { } else { args('--') } - args(dirsToUse.join(File.pathSeparator)) + args(getJavaClassesDir().absolutePath) } } doLast { successMarker.setText("", 'UTF-8') } } + File getJavaClassesDir() { + FileCollection classesDirs = (checkForTestsInMain ? + project.sourceSets.main.output.classesDirs : + project.sourceSets.test.output.classesDirs) + if (classesDirs.isEmpty()) { return null } + // see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html the deprecated property + // classesDir is the first in the FileCollection. There seems to be no other way to refer to the Java output dir + // We can't run the check against all classes as Groovy tests don't adhere to naming conventions. + return classesDirs[0] + } } diff --git a/server/build.gradle b/server/build.gradle index f9e3dad1be60a..ed3fe9d92df38 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -328,6 +328,7 @@ dependencyLicenses { if (isEclipse == false || project.path == ":server-tests") { task integTest(type: RandomizedTestingTask,group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' + configure(BuildPlugin.commonTestConfig(project)) basedOn tasks.test include '**/*IT.class' } diff --git a/x-pack/plugin/ml/build.gradle b/x-pack/plugin/ml/build.gradle index 116906d08b937..f5732824f4586 100644 --- a/x-pack/plugin/ml/build.gradle +++ b/x-pack/plugin/ml/build.gradle @@ -92,6 +92,7 @@ integTest.enabled = false // Instead we create a separate task to run the tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { + configure(BuildPlugin.commonTestConfig(project)) description = 'Multi-node tests' basedOn tasks.test include '**/*IT.class' diff --git a/x-pack/plugin/monitoring/build.gradle b/x-pack/plugin/monitoring/build.gradle index ae273e33d8335..3da79c8f74426 100644 --- a/x-pack/plugin/monitoring/build.gradle +++ b/x-pack/plugin/monitoring/build.gradle @@ -57,6 +57,7 @@ integTest.enabled = false // Instead we create a separate task to run the tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' + configure(BuildPlugin.commonTestConfig(project)) basedOn test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' diff --git a/x-pack/plugin/rollup/build.gradle b/x-pack/plugin/rollup/build.gradle index 7628d806ed273..821ac1e5e636b 100644 --- a/x-pack/plugin/rollup/build.gradle +++ b/x-pack/plugin/rollup/build.gradle @@ -37,6 +37,7 @@ integTest.enabled = false // Instead we create a separate task to run the tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' + configure(BuildPlugin.commonTestConfig(project)) basedOn tasks.test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' diff --git a/x-pack/plugin/upgrade/build.gradle b/x-pack/plugin/upgrade/build.gradle index 487d83c37ed94..588ad2c73c0ad 100644 --- a/x-pack/plugin/upgrade/build.gradle +++ b/x-pack/plugin/upgrade/build.gradle @@ -31,6 +31,7 @@ integTest.enabled = false // tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' + configure(BuildPlugin.commonTestConfig(project)) basedOn tasks.test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' From 2149a164e5098711d79996d748c0413bb0fcee0b Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 15 May 2018 12:37:01 +0300 Subject: [PATCH 03/19] Replace `add` with `addAll` This worked because the list is passed to `project.files` that does the right thing. --- .../org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy index 8e16b83cebc0b..ac1e12620af87 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LoggerUsageTask.groovy @@ -50,11 +50,11 @@ public class LoggerUsageTask extends LoggedExec { List files = [] // But only if the source sets that will make them exist if (project.sourceSets.findByName("main")) { - files.add(project.sourceSets.main.output.classesDirs.getFiles()) + files.addAll(project.sourceSets.main.output.classesDirs.getFiles()) dependsOn project.tasks.classes } if (project.sourceSets.findByName("test")) { - files.add(project.sourceSets.test.output.classesDirs.getFiles()) + files.addAll(project.sourceSets.test.output.classesDirs.getFiles()) dependsOn project.tasks.testClasses } /* In an extra twist, it isn't good enough that the source set From b8bf6510f2806787f868ccc231c8b3ba8d76b38f Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 15 May 2018 12:38:10 +0300 Subject: [PATCH 04/19] Revert "Only check Java tests, PR feedback" This reverts commit 9bd9389875d8b88aadb50df57a45cd0d2b073241. --- .../junit4/RandomizedTestingPlugin.groovy | 2 +- .../junit4/RandomizedTestingTask.groovy | 3 ++- .../precommit/NamingConventionsTask.groovy | 24 ++++++++----------- server/build.gradle | 1 - x-pack/plugin/ml/build.gradle | 1 - x-pack/plugin/monitoring/build.gradle | 1 - x-pack/plugin/rollup/build.gradle | 1 - x-pack/plugin/upgrade/build.gradle | 1 - 8 files changed, 13 insertions(+), 21 deletions(-) diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy index 06f7204027bc6..3a805b9902ab9 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy @@ -45,7 +45,7 @@ class RandomizedTestingPlugin implements Plugin { } static void replaceTestTask(Project project) { - TaskContainer tasks = project.tasks + def tasks = project.tasks Test oldTestTask = tasks.findByPath('test') if (oldTestTask == null) { // no test task, ok, user will use testing task on their own diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy index 954aed43f7260..9902ae7b1023b 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy @@ -98,6 +98,7 @@ class RandomizedTestingTask extends DefaultTask { } void basedOn(RandomizedTestingTask other) { + configure(BuildPlugin.commonTestConfig(project)) classpath = other.classpath; testClassesDirs = other.testClassesDirs; dependsOn = other.dependsOn @@ -223,7 +224,7 @@ class RandomizedTestingTask extends DefaultTask { DefaultLogger listener = null ByteArrayOutputStream antLoggingBuffer = null - if (false == logger.isInfoEnabled()) { + if (!logger.isInfoEnabled()) { // in info logging, ant already outputs info level, so we see everything // but on errors or when debugging, we want to see info level messages // because junit4 emits jvm output with ant logging diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy index ecad31db242c4..9450d62817c0e 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy @@ -67,7 +67,7 @@ public class NamingConventionsTask extends LoggedExec { NamingConventionsTask() { // Extra classpath contains the actual test - if (false == project.configurations.names.contains('namingConventions')) { + if (!project.configurations.names.contains('namingConventions')) { project.configurations.create('namingConventions') Dependency buildToolsDep = project.dependencies.add('namingConventions', "org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}") @@ -81,11 +81,17 @@ public class NamingConventionsTask extends LoggedExec { description = "Tests that test classes aren't misnamed or misplaced" executable = new File(project.runtimeJavaHome, 'bin/java') - if (false == checkForTestsInMain) { + def dirsToUse = (checkForTestsInMain ? + project.sourceSets.main.output.classesDirs : + project.sourceSets.test.output.classesDirs) + .filter {it.exists()} + .collect {it.getAbsolutePath()} + + if (!checkForTestsInMain) { /* This task is created by default for all subprojects with this * setting and there is no point in running it if the files don't * exist. */ - onlyIf { getJavaClassesDir() != null && getJavaClassesDir().exists() } + onlyIf { dirsToUse.size() != 0 } } /* @@ -118,20 +124,10 @@ public class NamingConventionsTask extends LoggedExec { } else { args('--') } - args(getJavaClassesDir().absolutePath) + args(dirsToUse.join(File.pathSeparator)) } } doLast { successMarker.setText("", 'UTF-8') } } - File getJavaClassesDir() { - FileCollection classesDirs = (checkForTestsInMain ? - project.sourceSets.main.output.classesDirs : - project.sourceSets.test.output.classesDirs) - if (classesDirs.isEmpty()) { return null } - // see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html the deprecated property - // classesDir is the first in the FileCollection. There seems to be no other way to refer to the Java output dir - // We can't run the check against all classes as Groovy tests don't adhere to naming conventions. - return classesDirs[0] - } } diff --git a/server/build.gradle b/server/build.gradle index ed3fe9d92df38..f9e3dad1be60a 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -328,7 +328,6 @@ dependencyLicenses { if (isEclipse == false || project.path == ":server-tests") { task integTest(type: RandomizedTestingTask,group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' - configure(BuildPlugin.commonTestConfig(project)) basedOn tasks.test include '**/*IT.class' } diff --git a/x-pack/plugin/ml/build.gradle b/x-pack/plugin/ml/build.gradle index f5732824f4586..116906d08b937 100644 --- a/x-pack/plugin/ml/build.gradle +++ b/x-pack/plugin/ml/build.gradle @@ -92,7 +92,6 @@ integTest.enabled = false // Instead we create a separate task to run the tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { - configure(BuildPlugin.commonTestConfig(project)) description = 'Multi-node tests' basedOn tasks.test include '**/*IT.class' diff --git a/x-pack/plugin/monitoring/build.gradle b/x-pack/plugin/monitoring/build.gradle index 3da79c8f74426..ae273e33d8335 100644 --- a/x-pack/plugin/monitoring/build.gradle +++ b/x-pack/plugin/monitoring/build.gradle @@ -57,7 +57,6 @@ integTest.enabled = false // Instead we create a separate task to run the tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' - configure(BuildPlugin.commonTestConfig(project)) basedOn test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' diff --git a/x-pack/plugin/rollup/build.gradle b/x-pack/plugin/rollup/build.gradle index 821ac1e5e636b..7628d806ed273 100644 --- a/x-pack/plugin/rollup/build.gradle +++ b/x-pack/plugin/rollup/build.gradle @@ -37,7 +37,6 @@ integTest.enabled = false // Instead we create a separate task to run the tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' - configure(BuildPlugin.commonTestConfig(project)) basedOn tasks.test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' diff --git a/x-pack/plugin/upgrade/build.gradle b/x-pack/plugin/upgrade/build.gradle index 588ad2c73c0ad..487d83c37ed94 100644 --- a/x-pack/plugin/upgrade/build.gradle +++ b/x-pack/plugin/upgrade/build.gradle @@ -31,7 +31,6 @@ integTest.enabled = false // tests based on ESIntegTestCase task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { description = 'Multi-node tests' - configure(BuildPlugin.commonTestConfig(project)) basedOn tasks.test include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' From f9bf8fa91f47db353e131e624b1188985fef6381 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 15 May 2018 12:45:54 +0300 Subject: [PATCH 05/19] Remove `basedOn` helper --- .../junit4/RandomizedTestingTask.groovy | 11 ----------- server/build.gradle | 12 +++++++++--- x-pack/plugin/ml/build.gradle | 19 +++++++++++++------ x-pack/plugin/monitoring/build.gradle | 19 +++++++++++++------ x-pack/plugin/rollup/build.gradle | 15 +++++++++++---- x-pack/plugin/upgrade/build.gradle | 16 +++++++++++----- 6 files changed, 57 insertions(+), 35 deletions(-) diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy index 9902ae7b1023b..138043165d341 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy @@ -97,17 +97,6 @@ class RandomizedTestingTask extends DefaultTask { listenersConfig.listeners.add(new TestReportLogger(logger: logger, config: testLoggingConfig)) } - void basedOn(RandomizedTestingTask other) { - configure(BuildPlugin.commonTestConfig(project)) - classpath = other.classpath; - testClassesDirs = other.testClassesDirs; - dependsOn = other.dependsOn - other.mustRunAfter this - if (project.tasks.findByPath("check") != null) { - project.tasks.check.dependsOn this - } - } - @Inject ProgressLoggerFactory getProgressLoggerFactory() { throw new UnsupportedOperationException() diff --git a/server/build.gradle b/server/build.gradle index f9e3dad1be60a..3055c625ea914 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -326,9 +326,15 @@ dependencyLicenses { } if (isEclipse == false || project.path == ":server-tests") { - task integTest(type: RandomizedTestingTask,group: JavaBasePlugin.VERIFICATION_GROUP) { - description = 'Multi-node tests' - basedOn tasks.test + task integTest(type: RandomizedTestingTask, + group: JavaBasePlugin.VERIFICATION_GROUP, + description: 'Multi-node tests', + dependsOn: test.dependsOn) { + configure(BuildPlugin.commonTestConfig(project)) + classpath = project.test.classpath + testClassesDirs = project.test.testClassesDirs include '**/*IT.class' } + check.dependsOn integTest + integTest.mustRunAfter test } diff --git a/x-pack/plugin/ml/build.gradle b/x-pack/plugin/ml/build.gradle index 116906d08b937..7f2b881ab9940 100644 --- a/x-pack/plugin/ml/build.gradle +++ b/x-pack/plugin/ml/build.gradle @@ -90,13 +90,20 @@ run { // so we disable integ tests integTest.enabled = false -// Instead we create a separate task to run the tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { - description = 'Multi-node tests' - basedOn tasks.test - include '**/*IT.class' - systemProperty 'es.set.netty.runtime.available.processors', 'false' +// Instead we create a separate task to run the +// tests based on ESIntegTestCase +task internalClusterTest(type: RandomizedTestingTask, + group: JavaBasePlugin.VERIFICATION_GROUP, + description: 'Multi-node tests', + dependsOn: test.dependsOn) { + configure(BuildPlugin.commonTestConfig(project)) + classpath = project.test.classpath + testClassesDirs = project.test.testClassesDirs + include '**/*IT.class' + systemProperty 'es.set.netty.runtime.available.processors', 'false' } +check.dependsOn internalClusterTest +internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { diff --git a/x-pack/plugin/monitoring/build.gradle b/x-pack/plugin/monitoring/build.gradle index ae273e33d8335..3fde6cd8c3775 100644 --- a/x-pack/plugin/monitoring/build.gradle +++ b/x-pack/plugin/monitoring/build.gradle @@ -54,13 +54,20 @@ run { // so we disable integ tests integTest.enabled = false -// Instead we create a separate task to run the tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { - description = 'Multi-node tests' - basedOn test - include '**/*IT.class' - systemProperty 'es.set.netty.runtime.available.processors', 'false' +// Instead we create a separate task to run the +// tests based on ESIntegTestCase +task internalClusterTest(type: RandomizedTestingTask, + group: JavaBasePlugin.VERIFICATION_GROUP, + description: 'Multi-node tests', + dependsOn: test.dependsOn) { + configure(BuildPlugin.commonTestConfig(project)) + classpath = project.test.classpath + testClassesDirs = project.test.testClassesDirs + include '**/*IT.class' + systemProperty 'es.set.netty.runtime.available.processors', 'false' } +check.dependsOn internalClusterTest +internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { task icTest { diff --git a/x-pack/plugin/rollup/build.gradle b/x-pack/plugin/rollup/build.gradle index 7628d806ed273..18ef7abee5c64 100644 --- a/x-pack/plugin/rollup/build.gradle +++ b/x-pack/plugin/rollup/build.gradle @@ -34,13 +34,20 @@ run { integTest.enabled = false -// Instead we create a separate task to run the tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { - description = 'Multi-node tests' - basedOn tasks.test +// Instead we create a separate task to run the +// tests based on ESIntegTestCase +task internalClusterTest(type: RandomizedTestingTask, + group: JavaBasePlugin.VERIFICATION_GROUP, + description: 'Multi-node tests', + dependsOn: test.dependsOn) { + configure(BuildPlugin.commonTestConfig(project)) + classpath = project.test.classpath + testClassesDirs = project.test.testClassesDirs include '**/*IT.class' systemProperty 'es.set.netty.runtime.available.processors', 'false' } +check.dependsOn internalClusterTest +internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { task icTest { diff --git a/x-pack/plugin/upgrade/build.gradle b/x-pack/plugin/upgrade/build.gradle index 487d83c37ed94..8e65f87da3070 100644 --- a/x-pack/plugin/upgrade/build.gradle +++ b/x-pack/plugin/upgrade/build.gradle @@ -29,12 +29,18 @@ integTest.enabled = false // Instead we create a separate task to run the // tests based on ESIntegTestCase -task internalClusterTest(type: RandomizedTestingTask, group: JavaBasePlugin.VERIFICATION_GROUP) { - description = 'Multi-node tests' - basedOn tasks.test - include '**/*IT.class' - systemProperty 'es.set.netty.runtime.available.processors', 'false' +task internalClusterTest(type: RandomizedTestingTask, + group: JavaBasePlugin.VERIFICATION_GROUP, + description: 'Multi-node tests', + dependsOn: test.dependsOn) { + configure(BuildPlugin.commonTestConfig(project)) + classpath = project.test.classpath + testClassesDirs = project.test.testClassesDirs + include '**/*IT.class' + systemProperty 'es.set.netty.runtime.available.processors', 'false' } +check.dependsOn internalClusterTest +internalClusterTest.mustRunAfter test // also add an "alias" task to make typing on the command line easier task icTest { From af3ac3e27c8753428f0044428192d551ab7c5db5 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 15 May 2018 13:31:06 +0300 Subject: [PATCH 06/19] Bring some changes back Previus revert accidentally reverted too much --- .../junit4/RandomizedTestingPlugin.groovy | 2 +- .../precommit/NamingConventionsTask.groovy | 24 +++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy index 3a805b9902ab9..06f7204027bc6 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingPlugin.groovy @@ -45,7 +45,7 @@ class RandomizedTestingPlugin implements Plugin { } static void replaceTestTask(Project project) { - def tasks = project.tasks + TaskContainer tasks = project.tasks Test oldTestTask = tasks.findByPath('test') if (oldTestTask == null) { // no test task, ok, user will use testing task on their own diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy index 9450d62817c0e..ecad31db242c4 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy @@ -67,7 +67,7 @@ public class NamingConventionsTask extends LoggedExec { NamingConventionsTask() { // Extra classpath contains the actual test - if (!project.configurations.names.contains('namingConventions')) { + if (false == project.configurations.names.contains('namingConventions')) { project.configurations.create('namingConventions') Dependency buildToolsDep = project.dependencies.add('namingConventions', "org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}") @@ -81,17 +81,11 @@ public class NamingConventionsTask extends LoggedExec { description = "Tests that test classes aren't misnamed or misplaced" executable = new File(project.runtimeJavaHome, 'bin/java') - def dirsToUse = (checkForTestsInMain ? - project.sourceSets.main.output.classesDirs : - project.sourceSets.test.output.classesDirs) - .filter {it.exists()} - .collect {it.getAbsolutePath()} - - if (!checkForTestsInMain) { + if (false == checkForTestsInMain) { /* This task is created by default for all subprojects with this * setting and there is no point in running it if the files don't * exist. */ - onlyIf { dirsToUse.size() != 0 } + onlyIf { getJavaClassesDir() != null && getJavaClassesDir().exists() } } /* @@ -124,10 +118,20 @@ public class NamingConventionsTask extends LoggedExec { } else { args('--') } - args(dirsToUse.join(File.pathSeparator)) + args(getJavaClassesDir().absolutePath) } } doLast { successMarker.setText("", 'UTF-8') } } + File getJavaClassesDir() { + FileCollection classesDirs = (checkForTestsInMain ? + project.sourceSets.main.output.classesDirs : + project.sourceSets.test.output.classesDirs) + if (classesDirs.isEmpty()) { return null } + // see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html the deprecated property + // classesDir is the first in the FileCollection. There seems to be no other way to refer to the Java output dir + // We can't run the check against all classes as Groovy tests don't adhere to naming conventions. + return classesDirs[0] + } } From f514cc8fa670b3251983f2a2bdd92561f1110edc Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 15 May 2018 13:34:15 +0300 Subject: [PATCH 07/19] Fix negation --- .../com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy index 138043165d341..2b61165608d2d 100644 --- a/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy +++ b/buildSrc/src/main/groovy/com/carrotsearch/gradle/junit4/RandomizedTestingTask.groovy @@ -213,7 +213,7 @@ class RandomizedTestingTask extends DefaultTask { DefaultLogger listener = null ByteArrayOutputStream antLoggingBuffer = null - if (!logger.isInfoEnabled()) { + if (logger.isInfoEnabled() == false) { // in info logging, ant already outputs info level, so we see everything // but on errors or when debugging, we want to see info level messages // because junit4 emits jvm output with ant logging From 32a64382dd3523157996b7fb7b63b113d9d05813 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 21 May 2018 10:05:31 +0300 Subject: [PATCH 08/19] add back public --- .../elasticsearch/gradle/precommit/NamingConventionsTask.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy index ecad31db242c4..d5615fa895e6c 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy @@ -65,7 +65,7 @@ public class NamingConventionsTask extends LoggedExec { @Input boolean checkForTestsInMain = false; - NamingConventionsTask() { + public NamingConventionsTask() { // Extra classpath contains the actual test if (false == project.configurations.names.contains('namingConventions')) { project.configurations.create('namingConventions') From a2800c0b363168339ea65e2a79ec8256e5883e6d Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 23 May 2018 14:45:07 +0300 Subject: [PATCH 09/19] revert name check changes --- .../test/NamingConventionsCheck.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java index bc5c1fb1b1ffc..9bd14675d34a4 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java +++ b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java @@ -19,7 +19,6 @@ package org.elasticsearch.test; -import java.io.File; import java.io.IOException; import java.lang.reflect.Modifier; import java.nio.file.FileVisitResult; @@ -31,7 +30,6 @@ import java.util.HashSet; import java.util.Objects; import java.util.Set; -import java.util.regex.Pattern; /** * Checks that all tests in a directory are named according to our naming conventions. This is important because tests that do not follow @@ -49,7 +47,7 @@ public class NamingConventionsCheck { public static void main(String[] args) throws IOException { Class testClass = null; Class integTestClass = null; - String rootPathList = null; + Path rootPath = null; boolean skipIntegTestsInDisguise = false; boolean selfTest = false; boolean checkMainClasses = false; @@ -72,7 +70,7 @@ public static void main(String[] args) throws IOException { checkMainClasses = true; break; case "--": - rootPathList = args[++i]; + rootPath = Paths.get(args[++i]); break; default: fail("unsupported argument '" + arg + "'"); @@ -80,13 +78,10 @@ public static void main(String[] args) throws IOException { } NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass); - for (String rootDir : rootPathList.split(Pattern.quote(File.pathSeparator))) { - Path rootPath = Paths.get(rootDir); - if (checkMainClasses) { - check.checkMain(rootPath); - } else { - check.checkTests(rootPath, skipIntegTestsInDisguise); - } + if (checkMainClasses) { + check.checkMain(rootPath); + } else { + check.checkTests(rootPath, skipIntegTestsInDisguise); } if (selfTest) { From 3bb77ff45e22d6e43cc52d590cdcbc35fe700d9d Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 28 May 2018 09:06:19 +0300 Subject: [PATCH 10/19] Revert "revert name check changes" This reverts commit a2800c0b363168339ea65e2a79ec8256e5883e6d. --- .../test/NamingConventionsCheck.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java index 9bd14675d34a4..bc5c1fb1b1ffc 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java +++ b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java @@ -19,6 +19,7 @@ package org.elasticsearch.test; +import java.io.File; import java.io.IOException; import java.lang.reflect.Modifier; import java.nio.file.FileVisitResult; @@ -30,6 +31,7 @@ import java.util.HashSet; import java.util.Objects; import java.util.Set; +import java.util.regex.Pattern; /** * 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 { public static void main(String[] args) throws IOException { Class testClass = null; Class integTestClass = null; - Path rootPath = null; + String rootPathList = null; boolean skipIntegTestsInDisguise = false; boolean selfTest = false; boolean checkMainClasses = false; @@ -70,7 +72,7 @@ public static void main(String[] args) throws IOException { checkMainClasses = true; break; case "--": - rootPath = Paths.get(args[++i]); + rootPathList = args[++i]; break; default: fail("unsupported argument '" + arg + "'"); @@ -78,10 +80,13 @@ public static void main(String[] args) throws IOException { } NamingConventionsCheck check = new NamingConventionsCheck(testClass, integTestClass); - if (checkMainClasses) { - check.checkMain(rootPath); - } else { - check.checkTests(rootPath, skipIntegTestsInDisguise); + for (String rootDir : rootPathList.split(Pattern.quote(File.pathSeparator))) { + Path rootPath = Paths.get(rootDir); + if (checkMainClasses) { + check.checkMain(rootPath); + } else { + check.checkTests(rootPath, skipIntegTestsInDisguise); + } } if (selfTest) { From 9f56f93c2aacfeac47032d9b7e8edceecb800b7a Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 28 May 2018 11:41:19 +0300 Subject: [PATCH 11/19] Pass all dirs to name check Only run on Java for build-tools, this is safe because it's a self test. It needs more work before we could pass in the Groovy classes as well as these inherit from `GroovyTestCase` --- buildSrc/build.gradle | 3 +++ .../precommit/NamingConventionsTask.groovy | 25 +++++++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 5256968b6ca3e..0f12e912b6e31 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -161,12 +161,15 @@ if (project != rootProject) { namingConventions { testClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$UnitTestCase' integTestClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$IntegTestCase' + // this is a self test, so should the classes dir change from this, it will fail + dirsToCheck = files("$buildDir/classes/java/test") } task namingConventionsMain(type: org.elasticsearch.gradle.precommit.NamingConventionsTask) { checkForTestsInMain = true testClass = namingConventions.testClass integTestClass = namingConventions.integTestClass + dirsToCheck = files("$buildDir/classes/java/main") } precommit.dependsOn namingConventionsMain } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy index d5615fa895e6c..f429e3bda253e 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy @@ -24,6 +24,7 @@ import org.elasticsearch.gradle.VersionProperties import org.gradle.api.artifacts.Dependency import org.gradle.api.file.FileCollection import org.gradle.api.tasks.Input +import org.gradle.api.tasks.Optional import org.gradle.api.tasks.OutputFile /** * Runs NamingConventionsCheck on a classpath/directory combo to verify that @@ -65,6 +66,15 @@ public class NamingConventionsTask extends LoggedExec { @Input boolean checkForTestsInMain = false; + /** + * Optional. The specific directories to check. Takes precedence in front of other options that control location. + * Allows the build script to specify exactly which location to test. + * Defaults to non empty directories in sourceSets.[test|main].outputClassesDirs + */ + @Input + @Optional + FileCollection dirsToCheck; + public NamingConventionsTask() { // Extra classpath contains the actual test if (false == project.configurations.names.contains('namingConventions')) { @@ -85,7 +95,7 @@ public class NamingConventionsTask extends LoggedExec { /* This task is created by default for all subprojects with this * setting and there is no point in running it if the files don't * exist. */ - onlyIf { getJavaClassesDir() != null && getJavaClassesDir().exists() } + onlyIf { getExistingClassesDirs().isEmpty() == false } } /* @@ -118,20 +128,19 @@ public class NamingConventionsTask extends LoggedExec { } else { args('--') } - args(getJavaClassesDir().absolutePath) + args(getExistingClassesDirs().getAsPath()) } } doLast { successMarker.setText("", 'UTF-8') } } - File getJavaClassesDir() { + FileCollection getExistingClassesDirs() { + if (dirsToCheck != null) { + return dirsToCheck + } FileCollection classesDirs = (checkForTestsInMain ? project.sourceSets.main.output.classesDirs : project.sourceSets.test.output.classesDirs) - if (classesDirs.isEmpty()) { return null } - // see https://docs.gradle.org/current/dsl/org.gradle.api.tasks.SourceSetOutput.html the deprecated property - // classesDir is the first in the FileCollection. There seems to be no other way to refer to the Java output dir - // We can't run the check against all classes as Groovy tests don't adhere to naming conventions. - return classesDirs[0] + return project.files(classesDirs.findAll { it.exists() }) } } From cb11667dd7d381a8c1b112ca28f7abb7e1aa78b2 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 30 May 2018 13:24:38 +0300 Subject: [PATCH 12/19] remove self tests from name check The self complicates the task setup and disable real checks on build-tools. With this change there are no more self tests, and the build-tools tests adhere to the conventions. The self test will be replaced by gradle test kit, thus the addition of the Gradle plugin builder plugin. --- buildSrc/build.gradle | 24 +- .../elasticsearch/gradle/LoggedExec.groovy | 45 ---- .../org/elasticsearch/gradle/LoggedExec.java | 44 ++++ .../gradle/VersionProperties.groovy | 41 ---- .../gradle/VersionProperties.java | 50 +++++ .../precommit/NamingConventionsTask.groovy | 146 ------------ .../precommit/NamingConventionsTask.java | 212 ++++++++++++++++++ .../gradle/vagrant/VagrantCommandTask.groovy | 5 - .../test/NamingConventionsCheck.java | 33 +-- ...t.groovy => VersionCollectionTests.groovy} | 12 +- .../doc/RestTestsFromSnippetsTaskTest.groovy | 32 ++- .../gradle/GradleIntegrationTests.java | 4 + .../gradle/GradleUnitTestCase.java | 4 + .../precommit/NamingConventionsTaskIT.java | 7 + .../namingConventionsSelfTest/build.gradle | 0 .../NamingConventionsCheckBadClasses.java | 2 +- .../test/NamingConventionsCheckInMainIT.java | 2 +- .../NamingConventionsCheckInMainTests.java | 2 +- 18 files changed, 373 insertions(+), 292 deletions(-) delete mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.groovy create mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java delete mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy create mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.java delete mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy create mode 100644 buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java rename buildSrc/src/test/groovy/org/elasticsearch/gradle/{VersionCollectionTest.groovy => VersionCollectionTests.groovy} (98%) create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java create mode 100644 buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle rename buildSrc/src/test/{ => resources/namingConventionsSelfTest/src/main}/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java (96%) rename buildSrc/src/{main => test/resources/namingConventionsSelfTest/src/test}/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java (92%) rename buildSrc/src/{main => test/resources/namingConventionsSelfTest/src/test}/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java (92%) diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 0f12e912b6e31..0ee66a2f843f0 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -21,7 +21,10 @@ import java.nio.file.Files import org.gradle.util.GradleVersion -apply plugin: 'groovy' +plugins { + id 'java-gradle-plugin' + id 'groovy' +} group = 'org.elasticsearch.gradle' @@ -83,7 +86,6 @@ repositories { } dependencies { - compile gradleApi() compile localGroovy() compile "com.carrotsearch.randomizedtesting:junit4-ant:${props.getProperty('randomizedrunner')}" compile("junit:junit:${props.getProperty('junit')}") { @@ -97,8 +99,10 @@ dependencies { compile 'de.thetaphi:forbiddenapis:2.5' compile 'org.apache.rat:apache-rat:0.11' compile "org.elasticsearch:jna:4.5.1" + testCompile "junit:junit:${props.getProperty('junit')}" } + // Gradle 2.14+ removed ProgressLogger(-Factory) classes from the public APIs // Use logging dependency instead // Gradle 4.3.1 stopped releasing the logging jars to jcenter, just use the last available one @@ -114,14 +118,12 @@ dependencies { *****************************************************************************/ // this will only happen when buildSrc is built on its own during build init if (project == rootProject) { - repositories { if (System.getProperty("repos.mavenLocal") != null) { mavenLocal() } mavenCentral() } - test.exclude 'org/elasticsearch/test/NamingConventionsCheckBadClasses*' } /***************************************************************************** @@ -159,17 +161,7 @@ if (project != rootProject) { } namingConventions { - testClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$UnitTestCase' - integTestClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$IntegTestCase' - // this is a self test, so should the classes dir change from this, it will fail - dirsToCheck = files("$buildDir/classes/java/test") - } - - task namingConventionsMain(type: org.elasticsearch.gradle.precommit.NamingConventionsTask) { - checkForTestsInMain = true - testClass = namingConventions.testClass - integTestClass = namingConventions.integTestClass - dirsToCheck = files("$buildDir/classes/java/main") + testClass = 'org.elasticsearch.gradle.GradleUnitTestCase' + integTestClass = 'org.elasticsearch.gradle.GradleIntegrationTests' } - precommit.dependsOn namingConventionsMain } diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.groovy deleted file mode 100644 index b1b04a2ded684..0000000000000 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.groovy +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.gradle - -import org.gradle.api.GradleException -import org.gradle.api.tasks.Exec - -/** - * A wrapper around gradle's Exec task to capture output and log on error. - */ -class LoggedExec extends Exec { - - protected ByteArrayOutputStream output = new ByteArrayOutputStream() - - LoggedExec() { - if (logger.isInfoEnabled() == false) { - standardOutput = output - errorOutput = output - ignoreExitValue = true - doLast { - if (execResult.exitValue != 0) { - output.toString('UTF-8').eachLine { line -> logger.error(line) } - throw new GradleException("Process '${executable} ${args.join(' ')}' finished with non-zero exit value ${execResult.exitValue}") - } - } - } - } -} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java new file mode 100644 index 0000000000000..d2e22b1c033ba --- /dev/null +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java @@ -0,0 +1,44 @@ +package org.elasticsearch.gradle; + +import groovy.lang.Closure; +import org.codehaus.groovy.runtime.DefaultGroovyMethods; +import org.codehaus.groovy.runtime.StringGroovyMethods; +import org.gradle.api.GradleException; +import org.gradle.api.Task; +import org.gradle.api.tasks.Exec; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.util.stream.Collectors; + +/** + * A wrapper around gradle's Exec task to capture output and log on error. + */ +public class LoggedExec extends Exec { + + protected ByteArrayOutputStream output = new ByteArrayOutputStream(); + + public LoggedExec() { + if (getLogger().isInfoEnabled() == false) { + setStandardOutput(output); + setErrorOutput(output); + setIgnoreExitValue(true); + doLast(new Closure(this, this) { + public void doCall(Task it) throws IOException { + if (getExecResult().getExitValue() != 0) { + for (String line : output.toString("UTF-8").split("\\R")) { + getLogger().error(line); + } + throw new GradleException( + "Process \'" + getExecutable() + " " + + getArgs().stream().collect(Collectors.joining(" "))+ + "\' finished with non-zero exit value " + + String.valueOf(getExecResult().getExitValue()) + ); + } + } + }); + } + } +} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy deleted file mode 100644 index 6983d12872f23..0000000000000 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.groovy +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.gradle - -/** - * Accessor for shared dependency versions used by elasticsearch, namely the elasticsearch and lucene versions. - */ -class VersionProperties { - static final Version elasticsearch - static final String lucene - static final Map versions = new HashMap<>() - static { - Properties props = new Properties() - InputStream propsStream = VersionProperties.class.getResourceAsStream('/version.properties') - if (propsStream == null) { - throw new RuntimeException('/version.properties resource missing') - } - props.load(propsStream) - elasticsearch = Version.fromString(props.getProperty('elasticsearch')) - lucene = props.getProperty('lucene') - for (String property : props.stringPropertyNames()) { - versions.put(property, props.getProperty(property)) - } - } -} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.java new file mode 100644 index 0000000000000..9ee597eb25ad8 --- /dev/null +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionProperties.java @@ -0,0 +1,50 @@ +package org.elasticsearch.gradle; + +import java.io.IOException; +import java.io.InputStream; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + +/** + * Accessor for shared dependency versions used by elasticsearch, namely the elasticsearch and lucene versions. + */ +public class VersionProperties { + public static Version getElasticsearch() { + return elasticsearch; + } + + public static String getLucene() { + return lucene; + } + + public static Map getVersions() { + return versions; + } + + private static final Version elasticsearch; + private static final String lucene; + private static final Map versions = new HashMap(); + static { + Properties props = getVersionProperties(); + elasticsearch = Version.fromString(props.getProperty("elasticsearch")); + lucene = props.getProperty("lucene"); + for (String property : props.stringPropertyNames()) { + versions.put(property, props.getProperty(property)); + } + } + + private static Properties getVersionProperties() { + Properties props = new Properties(); + InputStream propsStream = VersionProperties.class.getResourceAsStream("/version.properties"); + if (propsStream == null) { + throw new RuntimeException("/version.properties resource missing"); + } + try { + props.load(propsStream); + } catch (IOException e) { + throw new RuntimeException(e); + } + return props; + } +} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy deleted file mode 100644 index f429e3bda253e..0000000000000 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.groovy +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.elasticsearch.gradle.precommit - -import org.elasticsearch.gradle.LoggedExec -import org.elasticsearch.gradle.VersionProperties -import org.gradle.api.artifacts.Dependency -import org.gradle.api.file.FileCollection -import org.gradle.api.tasks.Input -import org.gradle.api.tasks.Optional -import org.gradle.api.tasks.OutputFile -/** - * Runs NamingConventionsCheck on a classpath/directory combo to verify that - * tests are named according to our conventions so they'll be picked up by - * gradle. Read the Javadoc for NamingConventionsCheck to learn more. - */ -public class NamingConventionsTask extends LoggedExec { - /** - * We use a simple "marker" file that we touch when the task succeeds - * as the task output. This is compared against the modified time of the - * inputs (ie the jars/class files). - */ - @OutputFile - File successMarker = new File(project.buildDir, "markers/${this.name}") - - /** - * Should we skip the integ tests in disguise tests? Defaults to true because only core names its - * integ tests correctly. - */ - @Input - boolean skipIntegTestInDisguise = false - - /** - * Superclass for all tests. - */ - @Input - String testClass = 'org.apache.lucene.util.LuceneTestCase' - - /** - * Superclass for all integration tests. - */ - @Input - String integTestClass = 'org.elasticsearch.test.ESIntegTestCase' - - /** - * Should the test also check the main classpath for test classes instead of - * doing the usual checks to the test classpath. - */ - @Input - boolean checkForTestsInMain = false; - - /** - * Optional. The specific directories to check. Takes precedence in front of other options that control location. - * Allows the build script to specify exactly which location to test. - * Defaults to non empty directories in sourceSets.[test|main].outputClassesDirs - */ - @Input - @Optional - FileCollection dirsToCheck; - - public NamingConventionsTask() { - // Extra classpath contains the actual test - if (false == project.configurations.names.contains('namingConventions')) { - project.configurations.create('namingConventions') - Dependency buildToolsDep = project.dependencies.add('namingConventions', - "org.elasticsearch.gradle:build-tools:${VersionProperties.elasticsearch}") - buildToolsDep.transitive = false // We don't need gradle in the classpath. It conflicts. - } - FileCollection classpath = project.files(project.configurations.namingConventions, - project.sourceSets.test.compileClasspath, - project.sourceSets.test.output) - dependsOn(classpath) - inputs.files(classpath) - description = "Tests that test classes aren't misnamed or misplaced" - executable = new File(project.runtimeJavaHome, 'bin/java') - - if (false == checkForTestsInMain) { - /* This task is created by default for all subprojects with this - * setting and there is no point in running it if the files don't - * exist. */ - onlyIf { getExistingClassesDirs().isEmpty() == false } - } - - /* - * We build the arguments in a funny afterEvaluate/doFirst closure so that we can wait for the classpath to be - * ready for us. Strangely neither one on their own are good enough. - */ - project.afterEvaluate { - doFirst { - args('-Djna.nosys=true') - args('-cp', classpath.asPath, 'org.elasticsearch.test.NamingConventionsCheck') - args('--test-class', testClass) - if (skipIntegTestInDisguise) { - args('--skip-integ-tests-in-disguise') - } else { - args('--integ-test-class', integTestClass) - } - /* - * The test framework has classes that fail the checks to validate that the checks fail properly. - * Since these would cause the build to fail we have to ignore them with this parameter. The - * process of ignoring them lets us validate that they were found so this ignore parameter acts - * as the test for the NamingConventionsCheck. - */ - if (':build-tools'.equals(project.path)) { - args('--self-test') - } - - if (checkForTestsInMain) { - args('--main') - args('--') - } else { - args('--') - } - args(getExistingClassesDirs().getAsPath()) - } - } - doLast { successMarker.setText("", 'UTF-8') } - } - - FileCollection getExistingClassesDirs() { - if (dirsToCheck != null) { - return dirsToCheck - } - FileCollection classesDirs = (checkForTestsInMain ? - project.sourceSets.main.output.classesDirs : - project.sourceSets.test.output.classesDirs) - return project.files(classesDirs.findAll { it.exists() }) - } -} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java new file mode 100644 index 0000000000000..b8f2a931a3a9f --- /dev/null +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java @@ -0,0 +1,212 @@ +package org.elasticsearch.gradle.precommit; + +import groovy.lang.Closure; +import org.codehaus.groovy.runtime.ResourceGroovyMethods; +import org.elasticsearch.gradle.LoggedExec; +import org.elasticsearch.gradle.VersionProperties; +import org.gradle.api.GradleException; +import org.gradle.api.Project; +import org.gradle.api.Task; +import org.gradle.api.artifacts.Dependency; +import org.gradle.api.artifacts.ModuleDependency; +import org.gradle.api.file.FileCollection; +import org.gradle.api.plugins.ExtraPropertiesExtension; +import org.gradle.api.plugins.JavaPluginConvention; +import org.gradle.api.tasks.AbstractExecTask; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Optional; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.SourceSetContainer; + +import java.io.File; +import java.io.IOException; +import java.util.Objects; + +/** + * Runs NamingConventionsCheck on a classpath/directory combo to verify that + * tests are named according to our conventions so they'll be picked up by + * gradle. Read the Javadoc for NamingConventionsCheck to learn more. + */ +public class NamingConventionsTask extends LoggedExec { + public NamingConventionsTask() { + setDescription("Tests that test classes aren't misnamed or misplaced"); + // Extra classpath contains the actual test + final Project project = getProject(); + if (project.getConfigurations().getNames().contains("namingConventions") == false) { + project.getConfigurations().create("namingConventions"); + Dependency buildToolsDep = project.getDependencies().add( + "namingConventions", + "org.elasticsearch.gradle:build-tools:" + VersionProperties.getElasticsearch() + ); + assert buildToolsDep instanceof ModuleDependency; + ((ModuleDependency) buildToolsDep).setTransitive(false); + } + + SourceSetContainer sourceSets = getJavaSourceSets(); + final FileCollection classpath = project.files( + project.getConfigurations().getByName("namingConventions"), + sourceSets.getByName("test").getCompileClasspath(), + sourceSets.getByName("test").getOutput() + ); + dependsOn(project.getTasks().matching(it -> "testCompileClasspath".equals(it.getName()))); + getInputs().files(classpath); + + setExecutable(new File( + Objects.requireNonNull( + project.getExtensions().getByType(ExtraPropertiesExtension.class).get("runtimeJavaHome") + ).toString(), + "bin/java") + ); + + if (checkForTestsInMain == false) { + /* This task is created by default for all subprojects with this + * setting and there is no point in running it if the files don't + * exist. */ + onlyIf((unused) -> getExistingClassesDirs().isEmpty() == false); + } + + /* + * We build the arguments in a funny afterEvaluate/doFirst closure so that we can wait for the classpath to be + * ready for us. Strangely neither one on their own are good enough. + */ + project.afterEvaluate(new Closure(this, this) { + public Task doCall(Project it) { + return doFirst(new Closure(NamingConventionsTask.this, NamingConventionsTask.this) { + public AbstractExecTask doCall(Task it) { + args("-Djna.nosys=true"); + args("-cp", classpath.getAsPath(), "org.elasticsearch.test.NamingConventionsCheck"); + args("--test-class", getTestClass()); + if (skipIntegTestInDisguise) { + args("--skip-integ-tests-in-disguise"); + } else { + args("--integ-test-class", getIntegTestClass()); + } + if (getCheckForTestsInMain()) { + args("--main"); + args("--"); + } else { + args("--"); + } + return args(getExistingClassesDirs().getAsPath()); + } + }); + } + }); + doLast(new Closure(this, this) { + public void doCall(Task it) { + try { + ResourceGroovyMethods.setText(getSuccessMarker(), "", "UTF-8"); + } catch (IOException e) { + throw new GradleException("io exception", e); + } + } + }); + } + + private SourceSetContainer getJavaSourceSets() { + return getProject().getConvention().getPlugin(JavaPluginConvention.class).getSourceSets(); + } + + public FileCollection getExistingClassesDirs() { + if (dirsToCheck != null) { + return dirsToCheck; + } + FileCollection classesDirs = getJavaSourceSets().getByName(checkForTestsInMain ? "main" : "test") + .getOutput().getClassesDirs(); + return classesDirs.filter(it -> it.exists()); + } + + public File getSuccessMarker() { + return successMarker; + } + + public void setSuccessMarker(File successMarker) { + this.successMarker = successMarker; + } + + public boolean getSkipIntegTestInDisguise() { + return skipIntegTestInDisguise; + } + + public boolean isSkipIntegTestInDisguise() { + return skipIntegTestInDisguise; + } + + public void setSkipIntegTestInDisguise(boolean skipIntegTestInDisguise) { + this.skipIntegTestInDisguise = skipIntegTestInDisguise; + } + + public String getTestClass() { + return testClass; + } + + public void setTestClass(String testClass) { + this.testClass = testClass; + } + + public String getIntegTestClass() { + return integTestClass; + } + + public void setIntegTestClass(String integTestClass) { + this.integTestClass = integTestClass; + } + + public boolean getCheckForTestsInMain() { + return checkForTestsInMain; + } + + public boolean isCheckForTestsInMain() { + return checkForTestsInMain; + } + + public void setCheckForTestsInMain(boolean checkForTestsInMain) { + this.checkForTestsInMain = checkForTestsInMain; + } + + public FileCollection getDirsToCheck() { + return dirsToCheck; + } + + public void setDirsToCheck(FileCollection dirsToCheck) { + this.dirsToCheck = dirsToCheck; + } + + /** + * We use a simple "marker" file that we touch when the task succeeds + * as the task output. This is compared against the modified time of the + * inputs (ie the jars/class files). + */ + @OutputFile + private File successMarker = new File(getProject().getBuildDir(), "markers/" + this.getName()); + /** + * Should we skip the integ tests in disguise tests? Defaults to true because only core names its + * integ tests correctly. + */ + @Input + private boolean skipIntegTestInDisguise = false; + /** + * Superclass for all tests. + */ + @Input + private String testClass = "org.apache.lucene.util.LuceneTestCase"; + /** + * Superclass for all integration tests. + */ + @Input + private String integTestClass = "org.elasticsearch.test.ESIntegTestCase"; + /** + * Should the test also check the main classpath for test classes instead of + * doing the usual checks to the test classpath. + */ + @Input + private boolean checkForTestsInMain = false; + /** + * Optional. The specific directories to check. Takes precedence in front of other options that control location. + * Allows the build script to specify exactly which location to test. + * Defaults to non empty directories in sourceSets.[test|main].outputClassesDirs + */ + @Input + @Optional + private FileCollection dirsToCheck; +} diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/vagrant/VagrantCommandTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/vagrant/VagrantCommandTask.groovy index aab120e8d049a..161584938bde8 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/vagrant/VagrantCommandTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/vagrant/VagrantCommandTask.groovy @@ -22,14 +22,9 @@ import org.apache.commons.io.output.TeeOutputStream import org.elasticsearch.gradle.LoggedExec import org.gradle.api.tasks.Input import org.gradle.api.tasks.Optional -import org.gradle.api.tasks.TaskAction import org.gradle.internal.logging.progress.ProgressLoggerFactory import javax.inject.Inject -import java.util.concurrent.CountDownLatch -import java.util.concurrent.locks.Lock -import java.util.concurrent.locks.ReadWriteLock -import java.util.concurrent.locks.ReentrantLock /** * Runs a vagrant command. Pretty much like Exec task but with a nicer output diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java index bc5c1fb1b1ffc..c69b95571d76c 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java +++ b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java @@ -39,11 +39,6 @@ * a class with a main method so gradle can call it for each project. This has the advantage of allowing gradle to calculate when it is * {@code UP-TO-DATE} so it can be skipped if the compiled classes haven't changed. This is useful on large modules for which checking all * the modules can be slow. - * - * Annoyingly, this cannot be tested using standard unit tests because to do so you'd have to declare classes that violate the rules. That - * would cause the test fail which would prevent the build from passing. So we have to make a mechanism for removing those test classes. Now - * that we have such a mechanism it isn't much work to fail the process if we don't detect the offending classes. Thus, the funky - * {@code --self-test} that is only run in the test:framework project. */ public class NamingConventionsCheck { public static void main(String[] args) throws IOException { @@ -51,7 +46,6 @@ public static void main(String[] args) throws IOException { Class integTestClass = null; String rootPathList = null; boolean skipIntegTestsInDisguise = false; - boolean selfTest = false; boolean checkMainClasses = false; for (int i = 0; i < args.length; i++) { String arg = args[i]; @@ -65,9 +59,6 @@ public static void main(String[] args) throws IOException { case "--skip-integ-tests-in-disguise": skipIntegTestsInDisguise = true; break; - case "--self-test": - selfTest = true; - break; case "--main": checkMainClasses = true; break; @@ -89,21 +80,6 @@ public static void main(String[] args) throws IOException { } } - if (selfTest) { - if (checkMainClasses) { - assertViolation(NamingConventionsCheckInMainTests.class.getName(), check.testsInMain); - assertViolation(NamingConventionsCheckInMainIT.class.getName(), check.testsInMain); - } else { - assertViolation("WrongName", check.missingSuffix); - assertViolation("WrongNameTheSecond", check.missingSuffix); - assertViolation("DummyAbstractTests", check.notRunnable); - assertViolation("DummyInterfaceTests", check.notRunnable); - assertViolation("InnerTests", check.innerClasses); - assertViolation("NotImplementingTests", check.notImplementing); - assertViolation("PlainUnit", check.pureUnitTest); - } - } - // Now we should have no violations assertNoViolations( "Not all subclasses of " + check.testClass.getSimpleName() @@ -143,7 +119,9 @@ public void checkTests(Path rootPath, boolean skipTestsInDisguised) throws IOExc Files.walkFileTree(rootPath, new TestClassVisitor() { @Override protected void visitTestClass(Class clazz) { - if (skipTestsInDisguised == false && integTestClass.isAssignableFrom(clazz)) { + if (skipTestsInDisguised == false && + integTestClass.isAssignableFrom(clazz) && + clazz != integTestClass) { integTestsInDisguise.add(clazz); } if (Modifier.isAbstract(clazz.getModifiers()) || Modifier.isInterface(clazz.getModifiers())) { @@ -259,15 +237,16 @@ abstract class TestClassVisitor implements FileVisitor { * Visit classes named like a test. */ protected abstract void visitTestClass(Class clazz); + /** * Visit classes named like an integration test. */ protected abstract void visitIntegrationTestClass(Class clazz); + /** * Visit classes not named like a test at all. */ protected abstract void visitOtherClass(Class clazz); - @Override public final FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException { // First we visit the root directory @@ -315,5 +294,7 @@ protected boolean isTestCase(Class clazz) { public final FileVisitResult visitFileFailed(Path file, IOException exc) throws IOException { throw exc; } + } + } diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTest.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy similarity index 98% rename from buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTest.groovy rename to buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy index 14f6d1b8523f7..82452da904dab 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTest.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy @@ -1,6 +1,10 @@ package org.elasticsearch.gradle -class VersionCollectionTest extends GroovyTestCase { +import org.junit.Test + +import static org.junit.Assert.* + +class VersionCollectionTests extends GradleUnitTestCase { String formatVersion(String version) { return " public static final Version V_${version.replaceAll("\\.", "_")} " @@ -16,6 +20,7 @@ class VersionCollectionTest extends GroovyTestCase { * branched from Major-1.x At the time of this writing 6.2 is unreleased and 6.3 is the 6.x branch. This test simulates the behavior * from 7.0 perspective, or master at the time of this writing. */ + @Test void testAgainstMajorUnreleasedWithExistingStagedMinorRelease() { VersionCollection vc = new VersionCollection(allVersions) assertNotNull(vc) @@ -51,6 +56,7 @@ class VersionCollectionTest extends GroovyTestCase { * unreleased minor is released. At the time of this writing 6.2 is unreleased, so adding a 6.2.1 simulates a 6.2 release. This test * simulates the behavior from 7.0 perspective, or master at the time of this writing. */ + @Test void testAgainstMajorUnreleasedWithoutStagedMinorRelease() { List localVersion = allVersions.clone() localVersion.add(formatVersion('6.2.1')) // release 6.2 @@ -89,6 +95,7 @@ class VersionCollectionTest extends GroovyTestCase { * branched from Major.x At the time of this writing 6.2 is unreleased and 6.3 is the 6.x branch. This test simulates the behavior * from 6.3 perspective. */ + @Test void testAgainstMinorReleasedBranch() { List localVersion = allVersions.clone() localVersion.removeAll { it.toString().contains('7_0_0')} // remove all the 7.x so that the actual version is 6.3 (6.x) @@ -126,6 +133,7 @@ class VersionCollectionTest extends GroovyTestCase { * unreleased minor is released. At the time of this writing 6.2 is unreleased, so adding a 6.2.1 simulates a 6.2 release. This test * simulates the behavior from 6.3 perspective. */ + @Test void testAgainstMinorReleasedBranchNoStagedMinor() { List localVersion = allVersions.clone() // remove all the 7.x and add a 6.2.1 which means 6.2 was released @@ -162,6 +170,7 @@ class VersionCollectionTest extends GroovyTestCase { * This validates the logic of being on a released minor branch. At the time of writing, 6.2 is unreleased, so this is equivalent of being * on 6.1. */ + @Test void testAgainstOldMinor() { List localVersion = allVersions.clone() @@ -195,6 +204,7 @@ class VersionCollectionTest extends GroovyTestCase { * This validates the lower bound of wire compat, which is 5.0. It also validates that the span of 2.x to 5.x if it is decided to port * this fix all the way to the maint 5.6 release. */ + @Test void testFloorOfWireCompatVersions() { List localVersion = [formatVersion('2.0.0'), formatVersion('2.0.1'), formatVersion('2.1.0'), formatVersion('2.1.1'), formatVersion('5.0.0'), formatVersion('5.0.1'), formatVersion('5.1.0'), formatVersion('5.1.1'), diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy index d0a7a2825e6f2..31c407a0a0e48 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy @@ -19,32 +19,46 @@ package org.elasticsearch.gradle.doc -import org.elasticsearch.gradle.doc.SnippetsTask.Snippet +import org.elasticsearch.gradle.GradleUnitTestCase import org.gradle.api.InvalidUserDataException +import org.junit.Rule +import org.junit.Test +import org.junit.rules.ExpectedException + +import static org.junit.Assert.* import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.replaceBlockQuote -class RestTestFromSnippetsTaskTest extends GroovyTestCase { +class RestTestFromSnippetsTaskTests extends GradleUnitTestCase { + + @Rule + public ExpectedException expectedEx = ExpectedException.none() + + @Test void testInvalidBlockQuote() { - String input = "\"foo\": \"\"\"bar\""; - String message = shouldFail({ replaceBlockQuote(input) }); - assertEquals("Invalid block quote starting at 7 in:\n$input", message); + String input = "\"foo\": \"\"\"bar\"" + expectedEx.expect(InvalidUserDataException.class) + expectedEx.expectMessage("Invalid block quote starting at 7 in:\n$input") + replaceBlockQuote(input) } + @Test void testSimpleBlockQuote() { assertEquals("\"foo\": \"bort baz\"", - replaceBlockQuote("\"foo\": \"\"\"bort baz\"\"\"")); + replaceBlockQuote("\"foo\": \"\"\"bort baz\"\"\"")) } + @Test void testMultipleBlockQuotes() { assertEquals("\"foo\": \"bort baz\", \"bar\": \"other\"", - replaceBlockQuote("\"foo\": \"\"\"bort baz\"\"\", \"bar\": \"\"\"other\"\"\"")); + replaceBlockQuote("\"foo\": \"\"\"bort baz\"\"\", \"bar\": \"\"\"other\"\"\"")) } + @Test void testEscapingInBlockQuote() { assertEquals("\"foo\": \"bort\\\" baz\"", - replaceBlockQuote("\"foo\": \"\"\"bort\" baz\"\"\"")); + replaceBlockQuote("\"foo\": \"\"\"bort\" baz\"\"\"")) assertEquals("\"foo\": \"bort\\n baz\"", - replaceBlockQuote("\"foo\": \"\"\"bort\n baz\"\"\"")); + replaceBlockQuote("\"foo\": \"\"\"bort\n baz\"\"\"")) } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java new file mode 100644 index 0000000000000..7b393dd1c2f00 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java @@ -0,0 +1,4 @@ +package org.elasticsearch.gradle; + +public class GradleIntegrationTests extends GradleUnitTestCase { +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java new file mode 100644 index 0000000000000..6d65579ff3171 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java @@ -0,0 +1,4 @@ +package org.elasticsearch.gradle; + +public abstract class GradleUnitTestCase { +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java new file mode 100644 index 0000000000000..9fce9d4215e6b --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -0,0 +1,7 @@ +package org.elasticsearch.gradle.precommit; + +import org.elasticsearch.gradle.GradleIntegrationTests; + +public abstract class NamingConventionsTaskIT extends GradleIntegrationTests { + +} diff --git a/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle b/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/buildSrc/src/test/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java b/buildSrc/src/test/resources/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java similarity index 96% rename from buildSrc/src/test/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java rename to buildSrc/src/test/resources/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java index 4fc88b3afc530..caac0a5ef69b4 100644 --- a/buildSrc/src/test/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java +++ b/buildSrc/src/test/resources/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.test; +package namingConventionsSelfTest.src.main.java.org.elasticsearch.test; import junit.framework.TestCase; diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java b/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java similarity index 92% rename from buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java rename to buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java index 46adc7f065b16..73d924d73e8c3 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java +++ b/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.test; +package namingConventionsSelfTest.src.test.java.org.elasticsearch.test; /** * This class should fail the naming conventions self test. diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java b/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java similarity index 92% rename from buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java rename to buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java index 27c0b41eb3f6a..1598198d15e69 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java +++ b/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.test; +package namingConventionsSelfTest.src.test.java.org.elasticsearch.test; /** * This class should fail the naming conventions self test. From a1f93cfc31a0d45da38f6ae51d3995588531b20b Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 30 May 2018 14:00:18 +0300 Subject: [PATCH 13/19] First test to run a Gradle build --- .../gradle/GradleIntegrationTests.java | 17 ++++++++++++++ .../precommit/NamingConventionsTaskIT.java | 23 ++++++++++++++++++- .../namingConventionsSelfTest/build.gradle | 19 +++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java index 7b393dd1c2f00..1c86c97b83cc2 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java @@ -1,4 +1,21 @@ package org.elasticsearch.gradle; +import org.junit.Test; + +import java.io.File; + public class GradleIntegrationTests extends GradleUnitTestCase { + + protected File getProjectDir(String name) { + File root = new File("src/test/resources/"); + if (root.exists() == false) { + throw new RuntimeException("Could not find resources dir for integration tests. " + + "Note that these tests can only be ran by Gradle and are not currently supported by the IDE"); + } + return new File(root, name); + } + + @Test + public void pass() { + } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index 9fce9d4215e6b..59cf700217a50 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -1,7 +1,28 @@ package org.elasticsearch.gradle.precommit; import org.elasticsearch.gradle.GradleIntegrationTests; +import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.GradleRunner; +import org.gradle.testkit.runner.TaskOutcome; +import org.junit.Assert; +import org.junit.Test; -public abstract class NamingConventionsTaskIT extends GradleIntegrationTests { +import java.io.File; + +import static org.junit.Assert.*; + +public class NamingConventionsTaskIT extends GradleIntegrationTests { + + @Test + public void pluginCanBeApplied() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("namingConventionsSelfTest")) + .withArguments("hello", "-s") + .withPluginClasspath() + .build(); + + assertTrue(result.getOutput().contains("build plugin can be applied")); + assertEquals(TaskOutcome.SUCCESS, result.task(":hello").getOutcome()); + } } diff --git a/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle b/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle index e69de29bb2d1d..cd09d93e1986a 100644 --- a/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle +++ b/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle @@ -0,0 +1,19 @@ +plugins { + id 'elasticsearch.build' +} + +dependencyLicenses.enabled = false +dependenciesInfo.enabled = false +forbiddenApisMain.enabled = false +forbiddenApisTest.enabled = false +jarHell.enabled = false +thirdPartyAudit.enabled = false + +ext.licenseFile = file("$buildDir/dummy/license") +ext.noticeFile = file("$buildDir/dummy/notice") + +task hello { + doFirst { + println "build plugin can be applied" + } +} From 4b6cb423dd96bfffbf4ee5bdb7de3047b5544fbf Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 30 May 2018 20:04:56 +0300 Subject: [PATCH 14/19] Add tests that replace the name check self test --- .../precommit/NamingConventionsTask.java | 41 +++--------- .../test/NamingConventionsCheck.java | 50 +++++++++------ .../gradle/GradleIntegrationTests.java | 2 +- .../precommit/NamingConventionsTaskIT.java | 63 +++++++++++++++++-- .../namingConventionsSelfTest/build.gradle | 11 ++++ .../NamingConventionsCheckBadClasses.java | 2 +- .../test/NamingConventionsCheckInMainIT.java | 7 ++- .../NamingConventionsCheckInMainTests.java | 2 +- .../org/elasticsearch/test/WrongName.java | 26 ++++++++ 9 files changed, 143 insertions(+), 61 deletions(-) rename buildSrc/src/{test/resources => testKit}/namingConventionsSelfTest/build.gradle (53%) rename buildSrc/src/{test/resources => testKit}/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java (96%) rename buildSrc/src/{test/resources => testKit}/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java (86%) rename buildSrc/src/{test/resources => testKit}/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java (92%) create mode 100644 buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/WrongName.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java index b8f2a931a3a9f..7b63899de31ee 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/NamingConventionsTask.java @@ -3,18 +3,15 @@ import groovy.lang.Closure; import org.codehaus.groovy.runtime.ResourceGroovyMethods; import org.elasticsearch.gradle.LoggedExec; -import org.elasticsearch.gradle.VersionProperties; +import org.elasticsearch.test.NamingConventionsCheck; import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.Task; -import org.gradle.api.artifacts.Dependency; -import org.gradle.api.artifacts.ModuleDependency; import org.gradle.api.file.FileCollection; import org.gradle.api.plugins.ExtraPropertiesExtension; import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.tasks.AbstractExecTask; import org.gradle.api.tasks.Input; -import org.gradle.api.tasks.Optional; import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.SourceSetContainer; @@ -30,21 +27,16 @@ public class NamingConventionsTask extends LoggedExec { public NamingConventionsTask() { setDescription("Tests that test classes aren't misnamed or misplaced"); - // Extra classpath contains the actual test final Project project = getProject(); - if (project.getConfigurations().getNames().contains("namingConventions") == false) { - project.getConfigurations().create("namingConventions"); - Dependency buildToolsDep = project.getDependencies().add( - "namingConventions", - "org.elasticsearch.gradle:build-tools:" + VersionProperties.getElasticsearch() - ); - assert buildToolsDep instanceof ModuleDependency; - ((ModuleDependency) buildToolsDep).setTransitive(false); - } SourceSetContainer sourceSets = getJavaSourceSets(); final FileCollection classpath = project.files( - project.getConfigurations().getByName("namingConventions"), + // This works because the class only depends on one class from junit that will be available from the + // tests compile classpath. It's the most straight forward way of telling Java where to find the main + // class. + NamingConventionsCheck.class.getProtectionDomain().getCodeSource().getLocation().getPath(), + // the tests to be loaded + checkForTestsInMain ? sourceSets.getByName("main").getRuntimeClasspath() : project.files(), sourceSets.getByName("test").getCompileClasspath(), sourceSets.getByName("test").getOutput() ); @@ -108,9 +100,6 @@ private SourceSetContainer getJavaSourceSets() { } public FileCollection getExistingClassesDirs() { - if (dirsToCheck != null) { - return dirsToCheck; - } FileCollection classesDirs = getJavaSourceSets().getByName(checkForTestsInMain ? "main" : "test") .getOutput().getClassesDirs(); return classesDirs.filter(it -> it.exists()); @@ -164,14 +153,6 @@ public void setCheckForTestsInMain(boolean checkForTestsInMain) { this.checkForTestsInMain = checkForTestsInMain; } - public FileCollection getDirsToCheck() { - return dirsToCheck; - } - - public void setDirsToCheck(FileCollection dirsToCheck) { - this.dirsToCheck = dirsToCheck; - } - /** * We use a simple "marker" file that we touch when the task succeeds * as the task output. This is compared against the modified time of the @@ -201,12 +182,4 @@ public void setDirsToCheck(FileCollection dirsToCheck) { */ @Input private boolean checkForTestsInMain = false; - /** - * Optional. The specific directories to check. Takes precedence in front of other options that control location. - * Allows the build script to specify exactly which location to test. - * Defaults to non empty directories in sourceSets.[test|main].outputClassesDirs - */ - @Input - @Optional - private FileCollection dirsToCheck; } diff --git a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java index c69b95571d76c..58e95cfc00232 100644 --- a/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java +++ b/buildSrc/src/main/java/org/elasticsearch/test/NamingConventionsCheck.java @@ -81,22 +81,39 @@ public static void main(String[] args) throws IOException { } // Now we should have no violations - assertNoViolations( + int exitCode = 0 ; + exitCode += countAndPrintViolations( "Not all subclasses of " + check.testClass.getSimpleName() + " match the naming convention. Concrete classes must end with [Tests]", - check.missingSuffix); - assertNoViolations("Classes ending with [Tests] are abstract or interfaces", check.notRunnable); - assertNoViolations("Found inner classes that are tests, which are excluded from the test runner", check.innerClasses); - assertNoViolations("Pure Unit-Test found must subclass [" + check.testClass.getSimpleName() + "]", check.pureUnitTest); - assertNoViolations("Classes ending with [Tests] must subclass [" + check.testClass.getSimpleName() + "]", check.notImplementing); - assertNoViolations( - "Classes ending with [Tests] or [IT] or extending [" + check.testClass.getSimpleName() + "] must be in src/test/java", - check.testsInMain); + check.missingSuffix) ; + exitCode += countAndPrintViolations( + "Classes ending with [Tests] are abstract or interfaces", + check.notRunnable + ); + exitCode += countAndPrintViolations( + "Found inner classes that are tests, which are excluded from the test runner", + check.innerClasses + ); + exitCode += countAndPrintViolations( + "Pure Unit-Test found must subclass [" + check.testClass.getSimpleName() + "]", + check.pureUnitTest + ); + exitCode += countAndPrintViolations( + "Classes ending with [Tests] must subclass [" + check.testClass.getSimpleName() + "]", + check.notImplementing + ); + exitCode += countAndPrintViolations( + "Classes ending with [Tests] or [IT] or extending [" + + check.testClass.getSimpleName() + "] must be in src/test/java", + check.testsInMain + ); if (skipIntegTestsInDisguise == false) { - assertNoViolations( - "Subclasses of " + check.integTestClass.getSimpleName() + " should end with IT as they are integration tests", - check.integTestsInDisguise); + exitCode += countAndPrintViolations("Subclasses of " + check.integTestClass.getSimpleName() + + " should end with IT as they are integration tests", + check.integTestsInDisguise + ); } + System.exit(exitCode); } private final Set> notImplementing = new HashSet<>(); @@ -179,18 +196,15 @@ protected void visitOtherClass(Class clazz) { } - /** - * Fail the process if there are any violations in the set. Named to look like a junit assertion even though it isn't because it is - * similar enough. - */ - private static void assertNoViolations(String message, Set> set) { + private static int countAndPrintViolations(String message, Set> set) { if (false == set.isEmpty()) { System.err.println(message + ":"); for (Class bad : set) { System.err.println(" * " + bad.getName()); } - System.exit(1); + return 1; } + return 0; } /** diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java index 1c86c97b83cc2..ca0bc4e3249ae 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java @@ -7,7 +7,7 @@ public class GradleIntegrationTests extends GradleUnitTestCase { protected File getProjectDir(String name) { - File root = new File("src/test/resources/"); + File root = new File("src/testKit/"); if (root.exists() == false) { throw new RuntimeException("Could not find resources dir for integration tests. " + "Note that these tests can only be ran by Gradle and are not currently supported by the IDE"); diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index 59cf700217a50..9de72be1b2c48 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -4,12 +4,14 @@ import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; -import org.junit.Assert; import org.junit.Test; -import java.io.File; +import java.util.Arrays; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; -import static org.junit.Assert.*; public class NamingConventionsTaskIT extends GradleIntegrationTests { @@ -17,12 +19,63 @@ public class NamingConventionsTaskIT extends GradleIntegrationTests { public void pluginCanBeApplied() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("namingConventionsSelfTest")) - .withArguments("hello", "-s") + .withArguments("hello", "-s", "-PcheckForTestsInMain=false") .withPluginClasspath() .build(); - assertTrue(result.getOutput().contains("build plugin can be applied")); assertEquals(TaskOutcome.SUCCESS, result.task(":hello").getOutcome()); + assertTrue(result.getOutput().contains("build plugin can be applied")); + } + + @Test + public void nameCheckFailsAsItShould() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("namingConventionsSelfTest")) + .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=false") + .withPluginClasspath() + .buildAndFail(); + + assertNotNull("task did not run", result.task(":namingConventions")); + assertEquals(TaskOutcome.FAILED, result.task(":namingConventions").getOutcome()); + for (String line : Arrays.asList( + "Found inner classes that are tests, which are excluded from the test runner:", + "* org.elasticsearch.test.NamingConventionsCheckInMainIT$InternalInvalidTests", + "Classes ending with [Tests] must subclass [UnitTestCase]:", + "* org.elasticsearch.test.NamingConventionsCheckInMainTests", + "* org.elasticsearch.test.NamingConventionsCheckInMainIT", + "Not all subclasses of UnitTestCase match the naming convention. Concrete classes must end with [Tests]:", + "* org.elasticsearch.test.WrongName")) { + assertTrue( + "expected: '" + line + "' but it was not found in the output", + result.getOutput().contains(line) + ); + } + } + + @Test + public void nameCheckFailsAsItShouldWithMain() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("namingConventionsSelfTest")) + .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=true") + .withPluginClasspath() + .buildAndFail(); + + assertNotNull("task did not run", result.task(":namingConventions")); + assertEquals(TaskOutcome.FAILED, result.task(":namingConventions").getOutcome()); + + for (String line : Arrays.asList( + "Classes ending with [Tests] or [IT] or extending [UnitTestCase] must be in src/test/java:", + "* org.elasticsearch.test.NamingConventionsCheckBadClasses$DummyInterfaceTests", + "* org.elasticsearch.test.NamingConventionsCheckBadClasses$DummyAbstractTests", + "* org.elasticsearch.test.NamingConventionsCheckBadClasses$InnerTests", + "* org.elasticsearch.test.NamingConventionsCheckBadClasses$NotImplementingTests", + "* org.elasticsearch.test.NamingConventionsCheckBadClasses$WrongNameTheSecond", + "* org.elasticsearch.test.NamingConventionsCheckBadClasses$WrongName")) { + assertTrue( + "expected: '" + line + "' but it was not found in the output", + result.getOutput().contains(line) + ); + } } } diff --git a/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle b/buildSrc/src/testKit/namingConventionsSelfTest/build.gradle similarity index 53% rename from buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle rename to buildSrc/src/testKit/namingConventionsSelfTest/build.gradle index cd09d93e1986a..47e0e94b86ac2 100644 --- a/buildSrc/src/test/resources/namingConventionsSelfTest/build.gradle +++ b/buildSrc/src/testKit/namingConventionsSelfTest/build.gradle @@ -1,4 +1,5 @@ plugins { + id 'java' id 'elasticsearch.build' } @@ -17,3 +18,13 @@ task hello { println "build plugin can be applied" } } + +dependencies { + compile "junit:junit:${versions.junit}" +} + +namingConventions { + checkForTestsInMain = project.property("checkForTestsInMain") == "true" + testClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$UnitTestCase' + integTestClass = 'org.elasticsearch.test.NamingConventionsCheckBadClasses$IntegTestCase' +} diff --git a/buildSrc/src/test/resources/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java b/buildSrc/src/testKit/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java similarity index 96% rename from buildSrc/src/test/resources/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java rename to buildSrc/src/testKit/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java index caac0a5ef69b4..4fc88b3afc530 100644 --- a/buildSrc/src/test/resources/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java +++ b/buildSrc/src/testKit/namingConventionsSelfTest/src/main/java/org/elasticsearch/test/NamingConventionsCheckBadClasses.java @@ -17,7 +17,7 @@ * under the License. */ -package namingConventionsSelfTest.src.main.java.org.elasticsearch.test; +package org.elasticsearch.test; import junit.framework.TestCase; diff --git a/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java b/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java similarity index 86% rename from buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java rename to buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java index 73d924d73e8c3..438f80154191b 100644 --- a/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java +++ b/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainIT.java @@ -17,10 +17,15 @@ * under the License. */ -package namingConventionsSelfTest.src.test.java.org.elasticsearch.test; +package org.elasticsearch.test; /** * This class should fail the naming conventions self test. */ public class NamingConventionsCheckInMainIT { + + public static class InternalInvalidTests extends NamingConventionsCheckBadClasses.UnitTestCase { + + } + } diff --git a/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java b/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java similarity index 92% rename from buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java rename to buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java index 1598198d15e69..27c0b41eb3f6a 100644 --- a/buildSrc/src/test/resources/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java +++ b/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/NamingConventionsCheckInMainTests.java @@ -17,7 +17,7 @@ * under the License. */ -package namingConventionsSelfTest.src.test.java.org.elasticsearch.test; +package org.elasticsearch.test; /** * This class should fail the naming conventions self test. diff --git a/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/WrongName.java b/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/WrongName.java new file mode 100644 index 0000000000000..64d6a237f8f4d --- /dev/null +++ b/buildSrc/src/testKit/namingConventionsSelfTest/src/test/java/org/elasticsearch/test/WrongName.java @@ -0,0 +1,26 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.test; + +/** + * This class should fail the naming conventions self test. + */ +public class WrongName extends NamingConventionsCheckBadClasses.UnitTestCase { +} From b7f8a1a5af9bcd896815e90e37768ce5245cabf1 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 25 Jun 2018 16:30:54 +0300 Subject: [PATCH 15/19] Clean up integ test base class --- ...tegrationTests.java => GradleIntegrationTestCase.java} | 8 ++------ .../gradle/precommit/NamingConventionsTaskIT.java | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) rename buildSrc/src/test/java/org/elasticsearch/gradle/{GradleIntegrationTests.java => GradleIntegrationTestCase.java} (77%) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTestCase.java similarity index 77% rename from buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java rename to buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTestCase.java index ca0bc4e3249ae..18a5f4e5e8e25 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTests.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTestCase.java @@ -1,10 +1,8 @@ package org.elasticsearch.gradle; -import org.junit.Test; - import java.io.File; -public class GradleIntegrationTests extends GradleUnitTestCase { +public abstract class GradleIntegrationTestCase { protected File getProjectDir(String name) { File root = new File("src/testKit/"); @@ -15,7 +13,5 @@ protected File getProjectDir(String name) { return new File(root, name); } - @Test - public void pass() { - } + } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index 9de72be1b2c48..c4e395f52151b 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -1,6 +1,6 @@ package org.elasticsearch.gradle.precommit; -import org.elasticsearch.gradle.GradleIntegrationTests; +import org.elasticsearch.gradle.GradleIntegrationTestCase; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; @@ -13,7 +13,7 @@ import static org.junit.Assert.assertTrue; -public class NamingConventionsTaskIT extends GradleIntegrationTests { +public class NamingConventionsTaskIT extends GradleIntegrationTestCase { @Test public void pluginCanBeApplied() { From c1798449a4bb81a8e09c86b26595a13d45805336 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 25 Jun 2018 16:31:07 +0300 Subject: [PATCH 16/19] Always run tests --- buildSrc/build.gradle | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 0ee66a2f843f0..0706f6f1e90d3 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -17,9 +17,8 @@ * under the License. */ -import java.nio.file.Files -import org.gradle.util.GradleVersion +import java.nio.file.Files plugins { id 'java-gradle-plugin' @@ -148,9 +147,6 @@ if (project != rootProject) { jarHell.enabled = false thirdPartyAudit.enabled = false - // test for elasticsearch.build tries to run with ES... - test.enabled = false - // TODO: re-enable once randomizedtesting gradle code is published and removed from here licenseHeaders.enabled = false @@ -162,6 +158,6 @@ if (project != rootProject) { namingConventions { testClass = 'org.elasticsearch.gradle.GradleUnitTestCase' - integTestClass = 'org.elasticsearch.gradle.GradleIntegrationTests' + integTestClass = 'org.elasticsearch.gradle.GradleIntegrationTestCase' } } From 1712976a73413c428aed5b775b7e08a44f4590d3 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 26 Jun 2018 08:43:07 +0300 Subject: [PATCH 17/19] Align with test naming conventions --- buildSrc/build.gradle | 6 +- .../gradle/VersionCollectionTests.groovy | 1 + ...y => RestTestFromSnippetsTaskTests.groovy} | 14 +---- .../gradle/GradleUnitTestCase.java | 4 -- .../precommit/NamingConventionsTaskIT.java | 12 ++-- .../gradle/test/BaseTestCase.java | 32 +++++++++++ .../{ => test}/GradleIntegrationTestCase.java | 4 +- .../gradle/test/GradleUnitTestCase.java | 4 ++ .../gradle/test/JUnit3MethodProvider.java | 55 +++++++++++++++++++ 9 files changed, 105 insertions(+), 27 deletions(-) rename buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/{RestTestsFromSnippetsTaskTest.groovy => RestTestFromSnippetsTaskTests.groovy} (90%) delete mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java rename buildSrc/src/test/java/org/elasticsearch/gradle/{ => test}/GradleIntegrationTestCase.java (79%) create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/test/JUnit3MethodProvider.java diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 0706f6f1e90d3..3d8131e310239 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -87,6 +87,8 @@ repositories { dependencies { compile localGroovy() compile "com.carrotsearch.randomizedtesting:junit4-ant:${props.getProperty('randomizedrunner')}" + compile "com.carrotsearch.randomizedtesting:randomizedtesting-runner:${props.getProperty('randomizedrunner')}" + compile("junit:junit:${props.getProperty('junit')}") { transitive = false } @@ -157,7 +159,7 @@ if (project != rootProject) { } namingConventions { - testClass = 'org.elasticsearch.gradle.GradleUnitTestCase' - integTestClass = 'org.elasticsearch.gradle.GradleIntegrationTestCase' + testClass = 'org.elasticsearch.gradle.test.GradleUnitTestCase' + integTestClass = 'org.elasticsearch.gradle.test.GradleIntegrationTestCase' } } diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy index 82452da904dab..57e8fecf67007 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy @@ -1,5 +1,6 @@ package org.elasticsearch.gradle +import org.elasticsearch.gradle.test.GradleUnitTestCase import org.junit.Test import static org.junit.Assert.* diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy similarity index 90% rename from buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy rename to buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy index 85c9b14fd0705..07831870be8d2 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestsFromSnippetsTaskTest.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy @@ -19,25 +19,20 @@ package org.elasticsearch.gradle.doc - -import org.elasticsearch.gradle.GradleUnitTestCase +import org.elasticsearch.gradle.test.GradleUnitTestCase import org.gradle.api.InvalidUserDataException import org.junit.Rule -import org.junit.Test import org.junit.rules.ExpectedException -import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.shouldAddShardFailureCheck import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.replaceBlockQuote -import static org.junit.Assert.assertEquals -import static org.junit.Assert.assertFalse -import static org.junit.Assert.assertTrue +import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.shouldAddShardFailureCheck +import static org.junit.Assert.* class RestTestFromSnippetsTaskTests extends GradleUnitTestCase { @Rule public ExpectedException expectedEx = ExpectedException.none() - @Test void testInvalidBlockQuote() { String input = "\"foo\": \"\"\"bar\"" expectedEx.expect(InvalidUserDataException.class) @@ -45,19 +40,16 @@ class RestTestFromSnippetsTaskTests extends GradleUnitTestCase { replaceBlockQuote(input) } - @Test void testSimpleBlockQuote() { assertEquals("\"foo\": \"bort baz\"", replaceBlockQuote("\"foo\": \"\"\"bort baz\"\"\"")) } - @Test void testMultipleBlockQuotes() { assertEquals("\"foo\": \"bort baz\", \"bar\": \"other\"", replaceBlockQuote("\"foo\": \"\"\"bort baz\"\"\", \"bar\": \"\"\"other\"\"\"")) } - @Test void testEscapingInBlockQuote() { assertEquals("\"foo\": \"bort\\\" baz\"", replaceBlockQuote("\"foo\": \"\"\"bort\" baz\"\"\"")) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java deleted file mode 100644 index 6d65579ff3171..0000000000000 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleUnitTestCase.java +++ /dev/null @@ -1,4 +0,0 @@ -package org.elasticsearch.gradle; - -public abstract class GradleUnitTestCase { -} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index c4e395f52151b..b2d81a7e21741 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -1,10 +1,9 @@ package org.elasticsearch.gradle.precommit; -import org.elasticsearch.gradle.GradleIntegrationTestCase; +import org.elasticsearch.gradle.test.GradleIntegrationTestCase; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; -import org.junit.Test; import java.util.Arrays; @@ -15,8 +14,7 @@ public class NamingConventionsTaskIT extends GradleIntegrationTestCase { - @Test - public void pluginCanBeApplied() { + public void testPluginCanBeApplied() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("namingConventionsSelfTest")) .withArguments("hello", "-s", "-PcheckForTestsInMain=false") @@ -27,8 +25,7 @@ public void pluginCanBeApplied() { assertTrue(result.getOutput().contains("build plugin can be applied")); } - @Test - public void nameCheckFailsAsItShould() { + public void testNameCheckFailsAsItShould() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("namingConventionsSelfTest")) .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=false") @@ -52,8 +49,7 @@ public void nameCheckFailsAsItShould() { } } - @Test - public void nameCheckFailsAsItShouldWithMain() { + public void testNameCheckFailsAsItShouldWithMain() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("namingConventionsSelfTest")) .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=true") diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java new file mode 100644 index 0000000000000..efc999cb35a7e --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java @@ -0,0 +1,32 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.gradle.test; + +import com.carrotsearch.randomizedtesting.JUnit4MethodProvider; +import com.carrotsearch.randomizedtesting.RandomizedRunner; +import com.carrotsearch.randomizedtesting.annotations.TestMethodProviders; +import org.junit.runner.RunWith; + +@RunWith(RandomizedRunner.class) +@TestMethodProviders({ + JUnit4MethodProvider.class, + JUnit3MethodProvider.class +}) +public class BaseTestCase { +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java similarity index 79% rename from buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTestCase.java rename to buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java index 18a5f4e5e8e25..150932f8bb79f 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -1,8 +1,8 @@ -package org.elasticsearch.gradle; +package org.elasticsearch.gradle.test; import java.io.File; -public abstract class GradleIntegrationTestCase { +public abstract class GradleIntegrationTestCase extends BaseTestCase { protected File getProjectDir(String name) { File root = new File("src/testKit/"); diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java new file mode 100644 index 0000000000000..b4ff234c5db68 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java @@ -0,0 +1,4 @@ +package org.elasticsearch.gradle.test; + +public abstract class GradleUnitTestCase extends BaseTestCase { +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/JUnit3MethodProvider.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/JUnit3MethodProvider.java new file mode 100644 index 0000000000000..18871e16555ef --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/JUnit3MethodProvider.java @@ -0,0 +1,55 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.gradle.test; + +import com.carrotsearch.randomizedtesting.ClassModel; +import com.carrotsearch.randomizedtesting.ClassModel.MethodModel; +import com.carrotsearch.randomizedtesting.TestMethodProvider; + +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Map; + +/** + * Backwards compatible test* method provider (public, non-static). + * + * copy of org.apache.lucene.util.LuceneJUnit3MethodProvider to avoid a dependency between build and test fw. + */ +public final class JUnit3MethodProvider implements TestMethodProvider { + @Override + public Collection getTestMethods(Class suiteClass, ClassModel classModel) { + Map methods = classModel.getMethods(); + ArrayList result = new ArrayList<>(); + for (MethodModel mm : methods.values()) { + // Skip any methods that have overrieds/ shadows. + if (mm.getDown() != null) continue; + + Method m = mm.element; + if (m.getName().startsWith("test") && + Modifier.isPublic(m.getModifiers()) && + !Modifier.isStatic(m.getModifiers()) && + m.getParameterTypes().length == 0) { + result.add(m); + } + } + return result; + } +} From 9578cb86ba01850c5aa1cb16725954a7607d94c6 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 26 Jun 2018 09:24:53 +0300 Subject: [PATCH 18/19] Make integ. test case inherit from unit test case The check requires this --- .../groovy/org/elasticsearch/gradle/LoggedExec.java | 3 --- .../org/elasticsearch/gradle/test/BaseTestCase.java | 2 +- .../gradle/test/GradleIntegrationTestCase.java | 3 +-- .../elasticsearch/gradle/test/GradleUnitTestCase.java | 10 ++++++++++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java b/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java index d2e22b1c033ba..7f51c4fb3987d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/LoggedExec.java @@ -1,15 +1,12 @@ package org.elasticsearch.gradle; import groovy.lang.Closure; -import org.codehaus.groovy.runtime.DefaultGroovyMethods; -import org.codehaus.groovy.runtime.StringGroovyMethods; import org.gradle.api.GradleException; import org.gradle.api.Task; import org.gradle.api.tasks.Exec; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.util.stream.Collectors; /** diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java index efc999cb35a7e..eb282f71ede4b 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java @@ -28,5 +28,5 @@ JUnit4MethodProvider.class, JUnit3MethodProvider.class }) -public class BaseTestCase { +public abstract class BaseTestCase { } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java index 150932f8bb79f..26da663182f7c 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -2,7 +2,7 @@ import java.io.File; -public abstract class GradleIntegrationTestCase extends BaseTestCase { +public abstract class GradleIntegrationTestCase extends GradleUnitTestCase { protected File getProjectDir(String name) { File root = new File("src/testKit/"); @@ -13,5 +13,4 @@ protected File getProjectDir(String name) { return new File(root, name); } - } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java index b4ff234c5db68..b24624c7854b8 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleUnitTestCase.java @@ -1,4 +1,14 @@ package org.elasticsearch.gradle.test; +import com.carrotsearch.randomizedtesting.JUnit4MethodProvider; +import com.carrotsearch.randomizedtesting.RandomizedRunner; +import com.carrotsearch.randomizedtesting.annotations.TestMethodProviders; +import org.junit.runner.RunWith; + +@RunWith(RandomizedRunner.class) +@TestMethodProviders({ + JUnit4MethodProvider.class, + JUnit3MethodProvider.class +}) public abstract class GradleUnitTestCase extends BaseTestCase { } From 72fb7a9ae0d267a7ff42c54bfa25e3d7c61e2c45 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 28 Jun 2018 11:57:19 +0300 Subject: [PATCH 19/19] Remove `import static org.junit.Assert.*` --- .../org/elasticsearch/gradle/VersionCollectionTests.groovy | 2 -- .../gradle/doc/RestTestFromSnippetsTaskTests.groovy | 1 - .../gradle/precommit/NamingConventionsTaskIT.java | 5 ----- .../java/org/elasticsearch/gradle/test/BaseTestCase.java | 3 ++- 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy index 57e8fecf67007..2901acf65220a 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/VersionCollectionTests.groovy @@ -3,8 +3,6 @@ package org.elasticsearch.gradle import org.elasticsearch.gradle.test.GradleUnitTestCase import org.junit.Test -import static org.junit.Assert.* - class VersionCollectionTests extends GradleUnitTestCase { String formatVersion(String version) { diff --git a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy index 07831870be8d2..df20f542f9c39 100644 --- a/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy +++ b/buildSrc/src/test/groovy/org/elasticsearch/gradle/doc/RestTestFromSnippetsTaskTests.groovy @@ -26,7 +26,6 @@ import org.junit.rules.ExpectedException import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.replaceBlockQuote import static org.elasticsearch.gradle.doc.RestTestsFromSnippetsTask.shouldAddShardFailureCheck -import static org.junit.Assert.* class RestTestFromSnippetsTaskTests extends GradleUnitTestCase { diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index b2d81a7e21741..bdb251f0528ea 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -7,11 +7,6 @@ import java.util.Arrays; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - - public class NamingConventionsTaskIT extends GradleIntegrationTestCase { public void testPluginCanBeApplied() { diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java index eb282f71ede4b..48a62f8900fae 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/BaseTestCase.java @@ -21,6 +21,7 @@ import com.carrotsearch.randomizedtesting.JUnit4MethodProvider; import com.carrotsearch.randomizedtesting.RandomizedRunner; import com.carrotsearch.randomizedtesting.annotations.TestMethodProviders; +import org.junit.Assert; import org.junit.runner.RunWith; @RunWith(RandomizedRunner.class) @@ -28,5 +29,5 @@ JUnit4MethodProvider.class, JUnit3MethodProvider.class }) -public abstract class BaseTestCase { +public abstract class BaseTestCase extends Assert { }