Fix extractjar task ci (#33272)

Remove tasks to check license and notice and add build integration test instead.

Closes #33201

Author: alpar-t

build.gradle

@@ -20,16 +20,12 @@
 import com.github.jengelman.gradle.plugins.shadow.ShadowPlugin
 import org.apache.tools.ant.taskdefs.condition.Os
 import org.elasticsearch.gradle.BuildPlugin
-import org.elasticsearch.gradle.LoggedExec
 import org.elasticsearch.gradle.Version
 import org.elasticsearch.gradle.VersionCollection
 import org.elasticsearch.gradle.VersionProperties
 import org.elasticsearch.gradle.plugin.PluginBuildPlugin
 import org.gradle.plugins.ide.eclipse.model.SourceFolder
 
-import java.nio.file.Files
-import java.nio.file.Path
-
 plugins {
     id 'com.gradle.build-scan' version '1.13.2'
 }
@@ -580,62 +576,6 @@ wrapper {
     }
 }
 
-static void assertLinesInFile(final Path path, final List<String> expectedLines) {
-  final List<String> actualLines = Files.readAllLines(path)
-  int line = 0
-  for (final String expectedLine : expectedLines) {
-    final String actualLine = actualLines.get(line)
-    if (expectedLine != actualLine) {
-      throw new GradleException("expected line [${line + 1}] in [${path}] to be [${expectedLine}] but was [${actualLine}]")
-    }
-    line++
-  }
-}
-
-/*
- * Check that all generated JARs have our NOTICE.txt and an appropriate
- * LICENSE.txt in them. We configurate this in gradle but we'd like to
- * be extra paranoid.
- */
-subprojects { project ->
-  project.tasks.withType(Jar).whenTaskAdded { jarTask ->
-    final Task extract = project.task("extract${jarTask.name.capitalize()}", type: LoggedExec) {
-      dependsOn jarTask
-      ext.destination = project.buildDir.toPath().resolve("jar-extracted/${jarTask.name}")
-      commandLine "${->new File(rootProject.compilerJavaHome, 'bin/jar')}",
-          'xf', "${-> jarTask.outputs.files.singleFile}", 'META-INF/LICENSE.txt', 'META-INF/NOTICE.txt'
-      workingDir destination
-      onlyIf {jarTask.enabled}
-      doFirst {
-        project.delete(destination)
-        Files.createDirectories(destination)
-      }
-    }
-
-    final Task checkNotice = project.task("verify${jarTask.name.capitalize()}Notice") {
-      dependsOn extract
-      onlyIf {jarTask.enabled}
-      doLast {
-        final List<String> noticeLines = Files.readAllLines(project.noticeFile.toPath())
-        final Path noticePath = extract.destination.resolve('META-INF/NOTICE.txt')
-        assertLinesInFile(noticePath, noticeLines)
-      }
-    }
-    project.check.dependsOn checkNotice
-
-    final Task checkLicense = project.task("verify${jarTask.name.capitalize()}License") {
-      dependsOn extract
-      onlyIf {jarTask.enabled}
-      doLast {
-        final List<String> licenseLines = Files.readAllLines(project.licenseFile.toPath())
-        final Path licensePath = extract.destination.resolve('META-INF/LICENSE.txt')
-        assertLinesInFile(licensePath, licenseLines)
-      }
-    }
-    project.check.dependsOn checkLicense
-  }
-}
-
 /* Remove assemble/dependenciesInfo on all qa projects because we don't need to publish
  * artifacts for them. */
 gradle.projectsEvaluated {

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

@@ -56,6 +56,7 @@ import org.gradle.util.GradleVersion
 import java.nio.charset.StandardCharsets
 import java.time.ZoneOffset
 import java.time.ZonedDateTime
+
 /**
  * Encapsulates build configuration for elasticsearch projects.
  */
@@ -739,6 +740,7 @@ class BuildPlugin implements Plugin<Project> {
                     }
                     from(project.noticeFile.parent) {
                         include project.noticeFile.name
+                        rename { 'NOTICE.txt' }
                     }
                 }
             }

buildSrc/src/test/java/org/elasticsearch/gradle/BuildPluginIT.java

@@ -0,0 +1,76 @@
+/*
+ * 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.apache.commons.io.IOUtils;
+import org.elasticsearch.gradle.test.GradleIntegrationTestCase;
+import org.gradle.testkit.runner.BuildResult;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
+
+public class BuildPluginIT extends GradleIntegrationTestCase {
+
+    public void testPluginCanBeApplied() {
+        BuildResult result = getGradleRunner("elasticsearch.build")
+            .withArguments("hello", "-s")
+            .build();
+        assertTaskSuccessful(result, ":hello");
+        assertOutputContains("build plugin can be applied");
+    }
+
+    public void testCheckTask() {
+        BuildResult result = getGradleRunner("elasticsearch.build")
+            .withArguments("check", "assemble", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath())
+            .build();
+        assertTaskSuccessful(result, ":check");
+    }
+
+    public void testLicenseAndNotice() throws IOException {
+        BuildResult result = getGradleRunner("elasticsearch.build")
+            .withArguments("clean", "assemble", "-s", "-Dlocal.repo.path=" + getLocalTestRepoPath())
+            .build();
+
+        assertTaskSuccessful(result, ":assemble");
+
+        assertBuildFileExists(result, "elasticsearch.build", "distributions/elasticsearch.build.jar");
+
+        try (ZipFile zipFile = new ZipFile(new File(
+            getBuildDir("elasticsearch.build"), "distributions/elasticsearch.build.jar"
+        ))) {
+            ZipEntry licenseEntry = zipFile.getEntry("META-INF/LICENSE.txt");
+            ZipEntry noticeEntry = zipFile.getEntry("META-INF/NOTICE.txt");
+            assertNotNull("Jar does not have META-INF/LICENSE.txt", licenseEntry);
+            assertNotNull("Jar does not have META-INF/NOTICE.txt", noticeEntry);
+            try (
+                InputStream license = zipFile.getInputStream(licenseEntry);
+                InputStream notice = zipFile.getInputStream(noticeEntry)
+            ) {
+                assertEquals("this is a test license file", IOUtils.toString(license, StandardCharsets.UTF_8.name()));
+                assertEquals("this is a test notice file", IOUtils.toString(notice, StandardCharsets.UTF_8.name()));
+            }
+        }
+    }
+
+
+}

buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java

@@ -2,74 +2,57 @@
 
 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 java.util.Arrays;
+import java.util.HashSet;
 
 public class NamingConventionsTaskIT extends GradleIntegrationTestCase {
 
-    public void testPluginCanBeApplied() {
-        BuildResult result = GradleRunner.create()
-            .withProjectDir(getProjectDir("namingConventionsSelfTest"))
-            .withArguments("hello", "-s", "-PcheckForTestsInMain=false")
-            .withPluginClasspath()
-            .build();
-
-        assertEquals(TaskOutcome.SUCCESS, result.task(":hello").getOutcome());
-        String output = result.getOutput();
-        assertTrue(output, output.contains("build plugin can be applied"));
-    }
-
     public void testNameCheckFailsAsItShould() {
-        BuildResult result = GradleRunner.create()
-            .withProjectDir(getProjectDir("namingConventionsSelfTest"))
+        BuildResult result = getGradleRunner("namingConventionsSelfTest")
             .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=false")
-            .withPluginClasspath()
             .buildAndFail();
 
-        assertNotNull("task did not run", result.task(":namingConventions"));
-        assertEquals(TaskOutcome.FAILED, result.task(":namingConventions").getOutcome());
-        String output = result.getOutput();
-        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:\n" + output,
-                output.contains(line)
-            );
-        }
+        assertTaskFailed(result, ":namingConventions");
+        assertOutputContains(
+            result.getOutput(),
+            // TODO: java9 Set.of
+            new HashSet<>(
+                Arrays.asList(
+                    "Not all subclasses of UnitTestCase match the naming convention. Concrete classes must end with [Tests]:",
+                    "* org.elasticsearch.test.WrongName",
+                    "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"
+                )
+            )
+        );
     }
 
     public void testNameCheckFailsAsItShouldWithMain() {
-        BuildResult result = GradleRunner.create()
-            .withProjectDir(getProjectDir("namingConventionsSelfTest"))
+        BuildResult result = getGradleRunner("namingConventionsSelfTest")
             .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=true")
-            .withPluginClasspath()
             .buildAndFail();
 
-        assertNotNull("task did not run", result.task(":namingConventions"));
-        assertEquals(TaskOutcome.FAILED, result.task(":namingConventions").getOutcome());
-
-        String output = result.getOutput();
-        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:\n"+output,
-                output.contains(line)
-            );
-        }
+        assertTaskFailed(result, ":namingConventions");
+        assertOutputContains(
+            result.getOutput(),
+            // TODO: java9 Set.of
+            new HashSet<>(
+                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$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"
+                )
+            )
+        );
     }
 
 }

buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java

@@ -10,6 +10,7 @@
 import java.nio.file.Path;
 import java.util.List;
 import java.util.Objects;
+import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -47,6 +48,12 @@ protected void assertOutputContains(String output, String... lines) {
         }
     }
 
+    protected void assertOutputContains(String output, Set<String> lines) {
+        for (String line : lines) {
+            assertOutputContains(output, line);
+        }
+    }
+
     protected void assertOutputContains(String output, String line) {
         assertTrue(
             "Expected the following line in output:\n\n" + line + "\n\nOutput is:\n" + output,
@@ -82,7 +89,7 @@ private void assertTaskOutcome(BuildResult result, String taskName, TaskOutcome
                 "\n\nOutput is:\n" + result.getOutput());
         }
         assertEquals(
-            "Expected task to be successful but it was: " + task.getOutcome() +
+            "Expected task `" + taskName +"` to be successful but it was: " + task.getOutcome() +
                 taskOutcome + "\n\nOutput is:\n" + result.getOutput() ,
             taskOutcome,
             task.getOutcome()

buildSrc/src/testKit/elasticsearch.build/LICENSE

@@ -0,0 +1 @@
+this is a test license file

buildSrc/src/testKit/elasticsearch.build/NOTICE

@@ -0,0 +1 @@
+this is a test notice file

buildSrc/src/testKit/elasticsearch.build/build.gradle

@@ -0,0 +1,36 @@
+plugins {
+    id 'java'
+    id 'elasticsearch.build'
+}
+
+ext.licenseFile = file("LICENSE")
+ext.noticeFile = file("NOTICE")
+
+dependencies {
+    compile "junit:junit:${versions.junit}"
+    // missing classes in thirdparty audit
+    compile 'org.hamcrest:hamcrest-core:1.3'
+}
+
+repositories {
+    mavenCentral()
+    repositories {
+        maven {
+            url System.getProperty("local.repo.path")
+        }
+    }
+}
+
+// todo remove offending rules
+forbiddenApisMain.enabled = false
+forbiddenApisTest.enabled = false
+// requires dependency on testing fw
+jarHell.enabled = false
+// we don't have tests for now
+test.enabled = false
+
+task hello {
+    doFirst {
+        println "build plugin can be applied"
+    }
+}

buildSrc/src/testKit/elasticsearch.build/licenses/hamcrest-core-1.3.jar.sha1

@@ -0,0 +1 @@
+42a25dc3219429f0e5d060061f71acb49bf010a0

buildSrc/src/testKit/elasticsearch.build/licenses/hamcrest-core-LICENSE.txt

@@ -0,0 +1 @@
+2973d150c0dc1fefe998f834810d68f278ea58ec

buildSrc/src/testKit/elasticsearch.build/licenses/hamcrest-core-NOTICE.txt

@@ -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;
+
+/**
+ * This is just a test class
+ */
+public class SampleClass {
+
+}