Skip to content

Commit

Permalink
[JENKINS-65757] only use pluginFirst classloader when you have no oth…
Browse files Browse the repository at this point in the history
…er option (and never for guice!) (#151)

Remove a number of unused dependencies to avoid issues with jar-hell in Jenkins itself and add a full e2e test.

* Add a full e2e test using realistic class loading
* remove the need for maven / guice
FileFilter just joined the passed in excludes as a comma separated list
and added defaults if the list was empty or null, But the code already handles this so just call the Plex util directly
* httpcomponents was declared as a dependency but unused and bundled
* xml-apis was declared bundled and unused
* final clean up of unrequired stuff
* skip the instruction coverage as it is compiler specific
JDK11 and JDK8 compilers generate different bytecode and so asserting
the bytecode instruction coverage would need to be different on
potentially different compilers (e.g eclipse vs jdk 8 vs jdk11 vs ...)
* remove shade configuration - it did nothing.
the shade plugin was bound to a phase that did not exist in the hpi
packaging type.
even if it did it would be completely unclear what it did as the shade
plugin does not play nicely with hpis.
  • Loading branch information
jtnord committed Jun 17, 2021
1 parent d25cd61 commit e54a545
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 197 deletions.
197 changes: 3 additions & 194 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ THE SOFTWARE.
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>4.18</version>
<version>4.19</version>
</parent>

<artifactId>jacoco</artifactId>
Expand All @@ -44,8 +44,8 @@ THE SOFTWARE.
<jacoco.version>0.8.7</jacoco.version>
<!-- see https://www.jenkins.io/changelog-stable/ for changelog of the LTS releases -->
<jenkins.version>2.263.4</jenkins.version>
<maven.version>3.8.1</maven.version>
<powermock.version>2.0.7</powermock.version>
<!-- until the plugin-pom picks up a newer version -->
<jenkins-test-harness.version>1589.vc23fca066d5c</jenkins-test-harness.version>
<!-- Do not fail if tests are run without argLine -->
<argLine />
</properties>
Expand Down Expand Up @@ -113,117 +113,6 @@ THE SOFTWARE.
</repository>
</distributionManagement>

<!-- enforce versions of transitive dependencies to match RequireUpperBoundDeps rule -->
<dependencyManagement>
<dependencies>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<version>2.8.0</version>
</dependency>
<dependency>
<groupId>org.apache.ant</groupId>
<artifactId>ant</artifactId>
<version>1.10.9</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.12.0</version>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpclient</artifactId>
<version>4.5.13</version>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-core</artifactId>
<version>${maven.version}</version>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-embedder</artifactId>
<version>${maven.version}</version>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-repository-metadata</artifactId>
<version>${maven.version}</version>
</dependency>
<dependency>
<groupId>org.apache.maven.reporting</groupId>
<artifactId>maven-reporting-api</artifactId>
<version>3.0</version>
</dependency>
<dependency>
<groupId>org.apache.maven.doxia</groupId>
<artifactId>doxia-sink-api</artifactId>
<version>1.9.1</version>
</dependency>
<dependency>
<groupId>org.apache.maven.doxia</groupId>
<artifactId>doxia-site-renderer</artifactId>
<version>1.7.4</version>
</dependency>
<dependency>
<groupId>org.apache.maven.reporting</groupId>
<artifactId>maven-reporting-impl</artifactId>
<version>3.0.0</version>
</dependency>
<dependency>
<groupId>org.apache.maven.shared</groupId>
<artifactId>maven-shared-utils</artifactId>
<version>3.2.1</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-velocity</artifactId>
<version>1.2</version>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-aether-provider</artifactId>
<version>3.3.9</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-classworlds</artifactId>
<version>2.6.0</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-component-annotations</artifactId>
<version>2.1.0</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-container-default</artifactId>
<version>1.7.1</version>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-interpolation</artifactId>
<version>1.24</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>maven-plugin</artifactId>
<version>3.7</version>
</dependency>
<dependency>
<groupId>com.google.inject</groupId>
<artifactId>guice</artifactId>
<version>4.2.1</version>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>30.1-jre</version>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand All @@ -245,7 +134,6 @@ THE SOFTWARE.
<dependency>
<groupId>org.powermock</groupId>
<artifactId>powermock-module-junit4</artifactId>
<version>${powermock.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -257,70 +145,13 @@ THE SOFTWARE.
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<version>2.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>${jacoco.version}</version>
<!-- Exclude a few unused transitive dependencies of things that are not needed here to avoid clashes with other plugins -->
<exclusions>
<exclusion>
<groupId>org.apache.maven.doxia</groupId>
<artifactId>*</artifactId>
</exclusion>
<exclusion>
<groupId>org.codehaus.plexus</groupId>
<artifactId>*</artifactId>
</exclusion>
<exclusion>
<groupId>org.sonatype.plexus</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-plugin-api</artifactId>
<version>${maven.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-artifact</artifactId>
<version>${maven.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-model</artifactId>
<version>${maven.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-settings</artifactId>
<version>${maven.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.2.1</version>
</dependency>
<!-- Somehow we need org.w3c.dom.ElementTraversal, but do not get it from any of the dependencies
do not update to 2.x as it will not contain the class any more! -->
<dependency>
<groupId>xml-apis</groupId>
<artifactId>xml-apis</artifactId>
<version>1.4.01</version>
</dependency>
<dependency>
<groupId>org.apache.httpcomponents</groupId>
<artifactId>httpcore</artifactId>
<version>4.4.13</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>structs</artifactId>
Expand Down Expand Up @@ -452,27 +283,6 @@ THE SOFTWARE.
</pluginManagement>

<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<version>3.1.1</version>
<executions>
<execution>
<phase>insert-test</phase>
<goals>
<goal>shade</goal>
</goals>
<configuration>
<relocations>
<relocation>
<pattern>org.objectweb.asm</pattern>
<shadedPattern>org.kohsuke.asm3</shadedPattern>
</relocation>
</relocations>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
Expand Down Expand Up @@ -530,7 +340,6 @@ THE SOFTWARE.
</execution>
</executions>
<configuration>
<pluginFirstClassLoader>true</pluginFirstClassLoader>
<maskClasses>org.objectweb.asm.</maskClasses>
</configuration>
</plugin>
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/hudson/plugins/jacoco/ExecutionFileLoader.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.jacoco.core.data.ExecutionDataReader;
import org.jacoco.core.data.ExecutionDataStore;
import org.jacoco.core.data.SessionInfoStore;
import org.jacoco.maven.FileFilter;


public class ExecutionFileLoader implements Serializable {
Expand Down Expand Up @@ -122,9 +121,8 @@ private IBundleCoverage analyzeStructure() throws IOException {
excludes = ITEM_ZERO;
}

final FileFilter fileFilter = new FileFilter(Arrays.asList(includes), Arrays.asList(excludes));
try {
final List<File> filesToAnalyze = FileUtils.getFiles(classDirectory, fileFilter.getIncludes(), fileFilter.getExcludes());
final List<File> filesToAnalyze = FileUtils.getFiles(classDirectory, String.join(",", includes), String.join(",", excludes));
for (final File file : filesToAnalyze) {
analyzer.analyzeAll(file);
}
Expand Down
111 changes: 111 additions & 0 deletions src/test/java/hudson/plugins/jacoco/e2e/E2ETest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package hudson.plugins.jacoco.e2e;

import java.util.ArrayList;
import java.util.List;

import org.hamcrest.Description;
import org.hamcrest.TypeSafeDiagnosingMatcher;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.RealJenkinsRule;

import hudson.Functions;
import hudson.model.Descriptor;
import hudson.model.FreeStyleBuild;
import hudson.model.FreeStyleProject;
import hudson.plugins.jacoco.JacocoBuildAction;
import hudson.plugins.jacoco.JacocoPublisher;
import hudson.plugins.jacoco.model.Coverage;
import hudson.tasks.BatchFile;
import hudson.tasks.Builder;
import hudson.tasks.Publisher;
import hudson.tasks.Shell;
import hudson.util.DescribableList;

import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;

import static hudson.plugins.jacoco.e2e.E2ETest.CoverageMatcher.withCoverage;

public class E2ETest {

@Rule
public RealJenkinsRule rjr = new RealJenkinsRule();

@Test
public void simpleTest() throws Throwable {
rjr.then(r -> {
FreeStyleProject project = r.createFreeStyleProject();
project.getBuildersList().addAll(createJacocoProjectBuilders());
project.getPublishersList().add(new JacocoPublisher());

FreeStyleBuild build = r.buildAndAssertSuccess(project);

assertThat("plugin collected data", build.getLog(), containsString("Collecting JaCoCo coverage data"));

JacocoBuildAction action = build.getAction(JacocoBuildAction.class);
assertThat("Build has the Jacoco Action", action, notNullValue());

assertThat("incorrect branch coverage reported", action.getBranchCoverage(), withCoverage(0, 621, 621));
assertThat("incorrect class coverage reported", action.getClassCoverage(), withCoverage(7, 59, 66));
assertThat("incorrect complexity coverage reported", action.getComplexityScore(), withCoverage(19, 835, 854));
// different compilers can generate different instructions (e.g. java8 vs java 11.
// so just skip this for now as it seems brittle
// assertThat("incorrect instruction coverage reported", action.getInstructionCoverage(), withCoverage(229, 9013, 9242)); /* java 8* /
// assertThat("incorrect instruction coverage reported", action.getInstructionCoverage(), withCoverage(229, 9010 , 9239)); /* java 11 */
assertThat("incorrect line coverage reported", action.getLineCoverage(), withCoverage(53, 1860, 1913));
}
);
}

private static List<Builder> createJacocoProjectBuilders() {
String[] commands = { "git clone --branch jacoco-3.2.0 https://github.com/jenkinsci/jacoco-plugin.git .",
"mvn -P enable-jacoco -B -Dtest=ClassReportTest test" };
List<Builder> builders = new ArrayList<>();
if (Functions.isWindows()) {
for (String command : commands) {
builders.add(new BatchFile(command));
}
} else {
for (String command : commands) {
builders.add(new Shell(command));
}
}
return builders;
}

public static class CoverageMatcher extends TypeSafeDiagnosingMatcher<Coverage> {

private final int covered;
private final int missed;
private final int total;

private CoverageMatcher(int covered, int missed, int total) {
this.covered = covered;
this.missed = missed;
this.total = total;
}
@Override
public void describeTo(Description description) {
description.appendText(" with covered="+ covered);
description.appendText(" and missed="+ missed);
description.appendText(" and total="+ total);

}

@Override
protected boolean matchesSafely(Coverage coverage, Description mismatchDescription) {
mismatchDescription.appendText("Coverage with covered="+ coverage.getCovered());
mismatchDescription.appendText(" and missed="+ coverage.getMissed());
mismatchDescription.appendText(" and total="+ coverage.getTotal());

return coverage.getCovered() == covered &&
coverage.getMissed() == missed &&
coverage.getTotal() == total;
}

public static CoverageMatcher withCoverage(int covered, int missed, int total) {
return new CoverageMatcher(covered, missed, total);
}
}
}

0 comments on commit e54a545

Please sign in to comment.