Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix extractjar task ci #33272

Merged
merged 5 commits into from
Sep 3, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 3 additions & 65 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/


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 org.gradle.util.GradleVersion
import org.gradle.util.DistributionLocator
import org.apache.tools.ant.taskdefs.condition.Os
import org.apache.tools.ant.filters.ReplaceTokens

import java.nio.file.Files
import java.nio.file.Path
import java.security.MessageDigest

plugins {
id 'com.gradle.build-scan' version '1.13.2'
Expand Down Expand Up @@ -569,62 +563,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,8 @@ import org.apache.tools.ant.taskdefs.condition.Os
import org.eclipse.jgit.lib.Constants
import org.eclipse.jgit.lib.RepositoryBuilder
import org.elasticsearch.gradle.precommit.PrecommitTasks
import org.gradle.api.GradleException
import org.gradle.api.InvalidUserDataException
import org.gradle.api.JavaVersion
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.XmlProvider
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.Dependency
import org.gradle.api.artifacts.ModuleDependency
import org.gradle.api.artifacts.ModuleVersionIdentifier
import org.gradle.api.artifacts.ProjectDependency
import org.gradle.api.artifacts.ResolvedArtifact
import org.gradle.api.artifacts.SelfResolvingDependency
import org.gradle.api.*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert converting to wildcard imports...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Groovy has a different setting for this, I was under the impression that I already had it configured,
but looks like it was the case only for java.

import org.gradle.api.artifacts.*
import org.gradle.api.artifacts.dsl.RepositoryHandler
import org.gradle.api.execution.TaskExecutionGraph
import org.gradle.api.plugins.JavaPlugin
Expand Down Expand Up @@ -738,6 +726,7 @@ class BuildPlugin implements Plugin<Project> {
}
from(project.noticeFile.parent) {
include project.noticeFile.name
rename { 'NOTICE.txt' }
}
}
}
Expand Down
76 changes: 76 additions & 0 deletions buildSrc/src/test/java/org/elasticsearch/gradle/BuildPluginIT.java
Original file line number Diff line number Diff line change
@@ -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", noticeEntry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTICE -> NOTICE.txt

try (
InputStream licese = zipFile.getInputStream(licenseEntry);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: licese -> license

InputStream notice = zipFile.getInputStream(noticeEntry)
) {
assertEquals("this is a test license file", IOUtils.toString(licese, StandardCharsets.UTF_8.name()));
assertEquals("this is a test notice file", IOUtils.toString(notice, StandardCharsets.UTF_8.name()));
}
}
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
)
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions buildSrc/src/testKit/elasticsearch.build/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
this is a test license file
1 change: 1 addition & 0 deletions buildSrc/src/testKit/elasticsearch.build/NOTICE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
this is a test notice file
36 changes: 36 additions & 0 deletions buildSrc/src/testKit/elasticsearch.build/build.gradle
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
42a25dc3219429f0e5d060061f71acb49bf010a0
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
2973d150c0dc1fefe998f834810d68f278ea58ec
Empty file.
Empty file.
Loading