From 65983772ea6979b6befc07c3b70218378eb085ed Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 20 Aug 2018 18:29:56 +0300 Subject: [PATCH 1/7] Add test to reproduce #32985 --- .../gradle/BuildExamplePluginsIT.java | 8 ---- .../gradle/precommit/JarHellTaskIT.java | 39 +++++++++++++++++++ .../test/GradleIntegrationTestCase.java | 9 +++++ buildSrc/src/testKit/jarHell/build.gradle | 33 ++++++++++++++++ 4 files changed, 81 insertions(+), 8 deletions(-) create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java create mode 100644 buildSrc/src/testKit/jarHell/build.gradle diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/BuildExamplePluginsIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/BuildExamplePluginsIT.java index 9b63d6f45e06b..aca9906701150 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/BuildExamplePluginsIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/BuildExamplePluginsIT.java @@ -153,12 +153,4 @@ private Path writeBuildScript(String script) { } } - private String getLocalTestRepoPath() { - String property = System.getProperty("test.local-test-repo-path"); - Objects.requireNonNull(property, "test.local-test-repo-path not passed to tests"); - File file = new File(property); - assertTrue("Expected " + property + " to exist, but it did not!", file.exists()); - return file.getAbsolutePath(); - } - } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java new file mode 100644 index 0000000000000..9f455b989a9f4 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java @@ -0,0 +1,39 @@ +package org.elasticsearch.gradle.precommit; + +import org.elasticsearch.gradle.test.GradleIntegrationTestCase; +import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.GradleRunner; +import org.gradle.testkit.runner.TaskOutcome; + +/* + * 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. + */ +public class JarHellTaskIT extends GradleIntegrationTestCase { + + public void testModuleInfo() + { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("jarHell")) + .withArguments("clean", "jarHell", "-s", "-Dlocal.repo.path="+getLocalTestRepoPath()) + .withPluginClasspath() + .build(); + + assertEquals(TaskOutcome.SUCCESS, result.task(":jarHell").getOutcome()); + } + +} 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 f00ab406a6c10..9b556d9847098 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -9,6 +9,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -109,4 +110,12 @@ protected void assertBuildFileDoesNotExists(BuildResult result, String projectNa Files.exists(absPath) ); } + + protected String getLocalTestRepoPath() { + String property = System.getProperty("test.local-test-repo-path"); + Objects.requireNonNull(property, "test.local-test-repo-path not passed to tests"); + File file = new File(property); + assertTrue("Expected " + property + " to exist, but it did not!", file.exists()); + return file.getAbsolutePath(); + } } diff --git a/buildSrc/src/testKit/jarHell/build.gradle b/buildSrc/src/testKit/jarHell/build.gradle new file mode 100644 index 0000000000000..0a77f60e39cd0 --- /dev/null +++ b/buildSrc/src/testKit/jarHell/build.gradle @@ -0,0 +1,33 @@ +plugins { + id 'java' + id 'elasticsearch.build' +} + +dependencyLicenses.enabled = false +dependenciesInfo.enabled = false +forbiddenApisMain.enabled = false +forbiddenApisTest.enabled = false +thirdPartyAudit.enabled = false +ext.licenseFile = file("$buildDir/dummy/license") +ext.noticeFile = file("$buildDir/dummy/notice") + +repositories { + mavenCentral() + repositories { + maven { + url System.getProperty("local.repo.path") + } + } +} + +dependencies { + // Needed for the JarHell task + testCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}") { + exclude group: "org.apache.logging.log4j", module: "log4j-api" + } + // both contain module-info.class + testCompile 'org.apache.logging.log4j:log4j-api:2.10.0' + testCompile 'org.projectlombok:lombok:1.18.2' +} + +namingConventions.enabled = false From d870449d7ffa7a015d90a04839c4ccbfb4b80236 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 20 Aug 2018 18:30:50 +0300 Subject: [PATCH 2/7] Add property to run single integration test --- buildSrc/build.gradle | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 967c2e27ee8df..0f47bd45c7536 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -167,7 +167,13 @@ if (project != rootProject) { it.tasks.matching { it.name == 'publishNebulaPublicationToLocalTestRepository'} } exclude "**/*Tests.class" - include "**/*IT.class" + // We need a custom solution so it doesn't interfere with the test task + if ( System.hasProperty("integTest.single") ) { + exclude "**/*" + include "**/${System.getProperty("build.test.single")}*.class" + } else { + include "**/*IT.class" + } testClassesDirs = sourceSets.test.output.classesDirs classpath = sourceSets.test.runtimeClasspath inputs.dir(file("src/testKit")) From 6aabb430034ff538b5bdd3ad5e49ae45b3a6fbde Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 21 Aug 2018 08:52:38 +0300 Subject: [PATCH 3/7] Ignore module-info in JarHell checks --- .../org/elasticsearch/gradle/precommit/JarHellTaskIT.java | 2 +- .../src/main/java/org/elasticsearch/bootstrap/JarHell.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java index 9f455b989a9f4..3c3d39ce27416 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java @@ -25,7 +25,7 @@ */ public class JarHellTaskIT extends GradleIntegrationTestCase { - public void testModuleInfo() + public void testDoesNotFailOnModuleInfo() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("jarHell")) diff --git a/libs/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java b/libs/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java index e171daeb79b85..3de0ae5117e6a 100644 --- a/libs/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java +++ b/libs/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java @@ -255,6 +255,10 @@ public static void checkJavaVersion(String resource, String targetVersion) { } private static void checkClass(Map clazzes, String clazz, Path jarpath) { + if (clazz.equals("module-info") || clazz.endsWith(".module-info")) { + // Ignore jigsaw module descriptions + return; + } Path previous = clazzes.put(clazz, jarpath); if (previous != null) { if (previous.equals(jarpath)) { From 708bf263a9425a5f4ae7c58b90d49baf0b93a18d Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 21 Aug 2018 09:29:15 +0300 Subject: [PATCH 4/7] Add a test that asserts jar hell works --- .../gradle/precommit/JarHellTask.groovy | 10 +++++---- .../gradle/precommit/PrecommitTasks.groovy | 22 ++++++++++++------- .../gradle/precommit/JarHellTaskIT.java | 22 ++++++++++++++----- .../test/GradleIntegrationTestCase.java | 3 ++- buildSrc/src/testKit/jarHell/build.gradle | 20 ++++++++++++++--- .../java/org/apache/logging/log4j/Logger.java | 7 ++++++ 6 files changed, 63 insertions(+), 21 deletions(-) create mode 100644 buildSrc/src/testKit/jarHell/src/main/java/org/apache/logging/log4j/Logger.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/JarHellTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/JarHellTask.groovy index 656d5e0d35a9e..444d2af9fe32e 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/JarHellTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/JarHellTask.groovy @@ -21,8 +21,8 @@ package org.elasticsearch.gradle.precommit import org.elasticsearch.gradle.LoggedExec import org.gradle.api.file.FileCollection +import org.gradle.api.tasks.Classpath import org.gradle.api.tasks.OutputFile - /** * Runs CheckJarHell on a classpath. */ @@ -34,12 +34,14 @@ public class JarHellTask extends LoggedExec { * inputs (ie the jars/class files). */ @OutputFile - File successMarker = new File(project.buildDir, 'markers/jarHell') + File successMarker + + @Classpath + FileCollection classpath public JarHellTask() { + successMarker = new File(project.buildDir, 'markers/jarHell-' + getName()) project.afterEvaluate { - FileCollection classpath = project.sourceSets.test.runtimeClasspath - inputs.files(classpath) dependsOn(classpath) description = "Runs CheckJarHell on ${classpath}" executable = new File(project.runtimeJavaHome, 'bin/java') diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 42dc29df058c6..2c576051950f0 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -38,14 +38,14 @@ class PrecommitTasks { /** Adds a precommit task, which depends on non-test verification tasks. */ public static Task create(Project project, boolean includeDependencyLicenses) { List precommitTasks = [ - configureForbiddenApis(project), - configureCheckstyle(project), - configureNamingConventions(project), - project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class), - project.tasks.create('licenseHeaders', LicenseHeadersTask.class), - project.tasks.create('filepermissions', FilePermissionsTask.class), - project.tasks.create('jarHell', JarHellTask.class), - project.tasks.create('thirdPartyAudit', ThirdPartyAuditTask.class) + configureForbiddenApis(project), + configureCheckstyle(project), + configureNamingConventions(project), + project.tasks.create('forbiddenPatterns', ForbiddenPatternsTask.class), + project.tasks.create('licenseHeaders', LicenseHeadersTask.class), + project.tasks.create('filepermissions', FilePermissionsTask.class), + configureJarHell(project), + project.tasks.create('thirdPartyAudit', ThirdPartyAuditTask.class) ] // Configure it but don't add it as a dependency yet @@ -84,6 +84,12 @@ class PrecommitTasks { return project.tasks.create(precommitOptions) } + private static Task configureJarHell(Project project) { + Task task = project.tasks.create('jarHell', JarHellTask.class) + task.classpath = project.sourceSets.test.runtimeClasspath + return task + } + private static Task configureForbiddenApis(Project project) { project.pluginManager.apply(ForbiddenApisPlugin.class) project.forbiddenApis { diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java index 3c3d39ce27416..702585243c4cb 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java @@ -3,7 +3,6 @@ import org.elasticsearch.gradle.test.GradleIntegrationTestCase; import org.gradle.testkit.runner.BuildResult; import org.gradle.testkit.runner.GradleRunner; -import org.gradle.testkit.runner.TaskOutcome; /* * Licensed to Elasticsearch under one or more contributor @@ -25,15 +24,28 @@ */ public class JarHellTaskIT extends GradleIntegrationTestCase { - public void testDoesNotFailOnModuleInfo() - { + public void testDoesNotFailOnModuleInfo() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("jarHell")) - .withArguments("clean", "jarHell", "-s", "-Dlocal.repo.path="+getLocalTestRepoPath()) + .withArguments("clean", "jarHellModuleInfo", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath()) .withPluginClasspath() .build(); - assertEquals(TaskOutcome.SUCCESS, result.task(":jarHell").getOutcome()); + assertTaskSuccessfull(result, ":jarHellModuleInfo"); + } + + public void testJarHellDetected() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("jarHell")) + .withArguments("clean", "jarHell", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath()) + .withPluginClasspath() + .buildAndFail(); + + assertOutputContains( + result.getOutput(), + "Exception in thread \"main\" java.lang.IllegalStateException: jar hell!", + "class: org.apache.logging.log4j.Logger" + ); } } 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 9b556d9847098..3d3b1408043a1 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -70,7 +70,8 @@ protected void assertOutputDoesNotContain(String output, String... lines) { protected void assertTaskSuccessfull(BuildResult result, String taskName) { BuildTask task = result.task(taskName); if (task == null) { - fail("Expected task `" + taskName + "` to be successful, but it did not run"); + fail("Expected task `" + taskName + "` to be successful, but it did not run" + + "\n\nOutput is:\n" + result.getOutput()); } assertEquals( "Expected task to be successful but it was: " + task.getOutcome() + diff --git a/buildSrc/src/testKit/jarHell/build.gradle b/buildSrc/src/testKit/jarHell/build.gradle index 0a77f60e39cd0..2be1d5be1ebad 100644 --- a/buildSrc/src/testKit/jarHell/build.gradle +++ b/buildSrc/src/testKit/jarHell/build.gradle @@ -1,3 +1,5 @@ +import org.elasticsearch.gradle.precommit.JarHellTask + plugins { id 'java' id 'elasticsearch.build' @@ -20,14 +22,26 @@ repositories { } } +sourceSets { + moduleinfo { + + } +} + dependencies { // Needed for the JarHell task - testCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}") { + testCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}") + + moduleinfoCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}") { exclude group: "org.apache.logging.log4j", module: "log4j-api" } // both contain module-info.class - testCompile 'org.apache.logging.log4j:log4j-api:2.10.0' - testCompile 'org.projectlombok:lombok:1.18.2' + moduleinfoCompile 'org.apache.logging.log4j:log4j-api:2.10.0' + moduleinfoCompile 'org.projectlombok:lombok:1.18.2' +} + +task jarHellModuleInfo(type: JarHellTask) { + classpath = project.sourceSets.moduleinfo.runtimeClasspath } namingConventions.enabled = false diff --git a/buildSrc/src/testKit/jarHell/src/main/java/org/apache/logging/log4j/Logger.java b/buildSrc/src/testKit/jarHell/src/main/java/org/apache/logging/log4j/Logger.java new file mode 100644 index 0000000000000..a4332c664fa38 --- /dev/null +++ b/buildSrc/src/testKit/jarHell/src/main/java/org/apache/logging/log4j/Logger.java @@ -0,0 +1,7 @@ +package org.apache.logging.log4j; + +// Jar Hell ! +public class Logger { + +} + From 92dce88f2c2e920c0f26ec5446498ece08e83681 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 29 Aug 2018 13:39:14 +0300 Subject: [PATCH 5/7] Add unit test --- .../elasticsearch/bootstrap/JarHellTests.java | 31 ++++++++++++------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/libs/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java b/libs/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java index ced68bdf10558..95c56f94ee4e1 100644 --- a/libs/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java +++ b/libs/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java @@ -76,19 +76,26 @@ public void testDifferentJars() throws Exception { } } - public void testDifferentJars() throws Exception { + public void testModuleInfo() throws Exception { Path dir = createTempDir(); - Set jars = asSet(makeJar(dir, "foo.jar", null, "meta-info.class"), - makeJar(dir, "bar.jar", null, "DuplicateClass.class")); - try { - JarHell.checkJarHell(jars, logger::debug); - fail("did not get expected exception"); - } catch (IllegalStateException e) { - assertTrue(e.getMessage().contains("jar hell!")); - assertTrue(e.getMessage().contains("DuplicateClass")); - assertTrue(e.getMessage().contains("foo.jar")); - assertTrue(e.getMessage().contains("bar.jar")); - } + JarHell.checkJarHell( + asSet( + makeJar(dir, "foo.jar", null, "module-info.class"), + makeJar(dir, "bar.jar", null, "module-info.class") + ), + logger::debug + ); + } + + public void testModuleInfoPackage() throws Exception { + Path dir = createTempDir(); + JarHell.checkJarHell( + asSet( + makeJar(dir, "foo.jar", null, "foo/bar/module-info.class"), + makeJar(dir, "bar.jar", null, "foo/bar/module-info.class") + ), + logger::debug + ); } public void testDirsOnClasspath() throws Exception { From 41d4f1c45a1a59ca0e4bf86d85903c6d908d3416 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 29 Aug 2018 13:39:26 +0300 Subject: [PATCH 6/7] Remove unrelated change --- buildSrc/build.gradle | 7 ------- 1 file changed, 7 deletions(-) diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle index 329a857791810..57affa11d0bfc 100644 --- a/buildSrc/build.gradle +++ b/buildSrc/build.gradle @@ -166,13 +166,6 @@ if (project != rootProject) { it.tasks.matching { it.name == 'publishNebulaPublicationToLocalTestRepository'} } exclude "**/*Tests.class" - // We need a custom solution so it doesn't interfere with the test task - if ( System.hasProperty("integTest.single") ) { - exclude "**/*" - include "**/${System.getProperty("build.test.single")}*.class" - } else { - include "**/*IT.class" - } testClassesDirs = sourceSets.test.output.classesDirs classpath = sourceSets.test.runtimeClasspath inputs.dir(file("src/testKit")) From e4aba1c37a5cecdd3790ab9fc923ba5190de0943 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 29 Aug 2018 14:03:23 +0300 Subject: [PATCH 7/7] Adjust integration test to test that jarhell is ran with precommit --- ...portElasticsearchBuildResourcesTaskIT.java | 8 +++---- .../gradle/precommit/JarHellTaskIT.java | 13 ++-------- .../test/GradleIntegrationTestCase.java | 16 +++++++++---- buildSrc/src/testKit/jarHell/build.gradle | 24 +++---------------- 4 files changed, 21 insertions(+), 40 deletions(-) diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java index 98fea2ea15ab6..99afd0bcbe0ae 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java @@ -40,7 +40,7 @@ public void testUpToDateWithSourcesConfigured() { .withArguments("buildResources", "-s", "-i") .withPluginClasspath() .build(); - assertTaskSuccessfull(result, ":buildResources"); + assertTaskSuccessful(result, ":buildResources"); assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml"); @@ -61,8 +61,8 @@ public void testImplicitTaskDependencyCopy() { .withPluginClasspath() .build(); - assertTaskSuccessfull(result, ":buildResources"); - assertTaskSuccessfull(result, ":sampleCopyAll"); + assertTaskSuccessful(result, ":buildResources"); + assertTaskSuccessful(result, ":sampleCopyAll"); assertBuildFileExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle.xml"); // This is a side effect of compile time reference assertBuildFileExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle_suppressions.xml"); @@ -75,7 +75,7 @@ public void testImplicitTaskDependencyInputFileOfOther() { .withPluginClasspath() .build(); - assertTaskSuccessfull(result, ":sample"); + assertTaskSuccessful(result, ":sample"); assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml"); } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java index 702585243c4cb..03f2022bc66e8 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/JarHellTaskIT.java @@ -24,23 +24,14 @@ */ public class JarHellTaskIT extends GradleIntegrationTestCase { - public void testDoesNotFailOnModuleInfo() { - BuildResult result = GradleRunner.create() - .withProjectDir(getProjectDir("jarHell")) - .withArguments("clean", "jarHellModuleInfo", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath()) - .withPluginClasspath() - .build(); - - assertTaskSuccessfull(result, ":jarHellModuleInfo"); - } - public void testJarHellDetected() { BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("jarHell")) - .withArguments("clean", "jarHell", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath()) + .withArguments("clean", "precommit", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath()) .withPluginClasspath() .buildAndFail(); + assertTaskFailed(result, ":jarHell"); assertOutputContains( result.getOutput(), "Exception in thread \"main\" java.lang.IllegalStateException: jar hell!", 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 65f615a42d671..a1d4b86ab760c 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -67,16 +67,24 @@ protected void assertOutputDoesNotContain(String output, String... lines) { } } - protected void assertTaskSuccessfull(BuildResult result, String taskName) { + protected void assertTaskFailed(BuildResult result, String taskName) { + assertTaskOutcome(result, taskName, TaskOutcome.FAILED); + } + + protected void assertTaskSuccessful(BuildResult result, String taskName) { + assertTaskOutcome(result, taskName, TaskOutcome.SUCCESS); + } + + private void assertTaskOutcome(BuildResult result, String taskName, TaskOutcome taskOutcome) { BuildTask task = result.task(taskName); if (task == null) { - fail("Expected task `" + taskName + "` to be successful, but it did not run" + + fail("Expected task `" + taskName + "` to be " + taskOutcome +", but it did not run" + "\n\nOutput is:\n" + result.getOutput()); } assertEquals( "Expected task to be successful but it was: " + task.getOutcome() + - "\n\nOutput is:\n" + result.getOutput() , - TaskOutcome.SUCCESS, + taskOutcome + "\n\nOutput is:\n" + result.getOutput() , + taskOutcome, task.getOutcome() ); } diff --git a/buildSrc/src/testKit/jarHell/build.gradle b/buildSrc/src/testKit/jarHell/build.gradle index 2be1d5be1ebad..17ff43fc7403b 100644 --- a/buildSrc/src/testKit/jarHell/build.gradle +++ b/buildSrc/src/testKit/jarHell/build.gradle @@ -1,5 +1,3 @@ -import org.elasticsearch.gradle.precommit.JarHellTask - plugins { id 'java' id 'elasticsearch.build' @@ -10,6 +8,7 @@ dependenciesInfo.enabled = false forbiddenApisMain.enabled = false forbiddenApisTest.enabled = false thirdPartyAudit.enabled = false +namingConventions.enabled = false ext.licenseFile = file("$buildDir/dummy/license") ext.noticeFile = file("$buildDir/dummy/notice") @@ -22,26 +21,9 @@ repositories { } } -sourceSets { - moduleinfo { - - } -} - dependencies { // Needed for the JarHell task testCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}") - - moduleinfoCompile ("org.elasticsearch.test:framework:${versions.elasticsearch}") { - exclude group: "org.apache.logging.log4j", module: "log4j-api" - } - // both contain module-info.class - moduleinfoCompile 'org.apache.logging.log4j:log4j-api:2.10.0' - moduleinfoCompile 'org.projectlombok:lombok:1.18.2' -} - -task jarHellModuleInfo(type: JarHellTask) { - classpath = project.sourceSets.moduleinfo.runtimeClasspath + // causes jar hell with local sources + compile "org.apache.logging.log4j:log4j-api:${versions.log4j}" } - -namingConventions.enabled = false