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.

Sign up for GitHub

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

Jump to bottom

[JENKINS-27120] Adding Workflow support for JaCoCo publisher #66

Merged
merged 11 commits into from
May 30, 2016
Merged

[JENKINS-27120] Adding Workflow support for JaCoCo publisher #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c81f231
Manually merge changes from PR63 as the actual PR contains lots of un…
centic9 Jan 15, 2016
a41a986
Merge remote-tracking branch 'origin/master' into pr63_manualmerge
Feb 5, 2016
0810660
removes unused code
lordofthejars Feb 5, 2016
4e4fb1b
fixes wrong tests.
lordofthejars Feb 5, 2016
28a7f26
Merge pull request #69 from lordofthejars/pr63_manualmerge
centic9 Feb 5, 2016
ebdd68d
fix FreeStyle projects to work as expected
lordofthejars Feb 5, 2016
7ff656f
Merge pull request #70 from lordofthejars/pr63_manualmerge
centic9 Feb 8, 2016
b64da06
Merge branch 'master' into pr63_manualmerge
centic9 Feb 25, 2016
08bdee6
Revert some whitespace-changes
centic9 Apr 4, 2016
249bd6d
fixes code review issues
lordofthejars Apr 15, 2016
adc858d
Merge pull request #72 from lordofthejars/pr63_manualmerge
centic9 Apr 15, 2016
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
43 changes: 39 additions & 4 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ THE SOFTWARE.
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>1.424.6</version>
<relativePath />
<version>1.609.3</version>
Copy link
Member

Choose a reason for hiding this comment

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

BTW consider using the 2.x parent POM at some point. A lot more pleasant to work with. (The baseline would in that case be defined using the jenkins.version property.)

Copy link
Contributor

Choose a reason for hiding this comment

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

For now I will leave as is and if in the future we need to move forward, let's open an issue. Changing parent pom could generate any incompatibility? For what I see is that it is only going to work in Jenkins 2?

<relativePath/>
</parent>

<artifactId>jacoco</artifactId>
Expand Down Expand Up @@ -154,6 +154,41 @@ THE SOFTWARE.
<artifactId>jacoco-maven-plugin</artifactId>
<version>${jacoco.version}</version>
</dependency>
<dependency>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-plugin-plugin</artifactId>
<version>3.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-plugin-api</artifactId>
<version>3.3.9</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-artifact</artifactId>
<version>3.3.9</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-model</artifactId>
<version>3.3.9</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.maven</groupId>
<artifactId>maven-settings</artifactId>
<version>3.3.9</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<version>3.0.15</version>
</dependency>
</dependencies>

<distributionManagement>
Expand Down Expand Up @@ -191,8 +226,8 @@ THE SOFTWARE.
<artifactId>maven-compiler-plugin</artifactId>
<version>2.3.2</version>
<configuration>
<source>1.5</source>
<target>1.5</target>
<source>1.6</source>
<target>1.6</target>
</configuration>
</plugin>
<plugin>
Expand Down
42 changes: 23 additions & 19 deletions src/main/java/hudson/plugins/jacoco/JacocoBuildAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import java.util.LinkedHashMap;
import java.util.Map;

import hudson.model.Run;
import hudson.model.TaskListener;
import jenkins.model.RunAction2;
import org.jacoco.core.analysis.IBundleCoverage;
import org.jvnet.localizer.Localizable;
import org.kohsuke.stapler.StaplerProxy;
Expand All @@ -33,9 +36,9 @@
* @author Jonathan Fuerth
* @author Ognjen Bubalo
*/
public final class JacocoBuildAction extends CoverageObject<JacocoBuildAction> implements HealthReportingAction, StaplerProxy, Serializable {
public final class JacocoBuildAction extends CoverageObject<JacocoBuildAction> implements HealthReportingAction, StaplerProxy, Serializable, RunAction2 {
Copy link
Member

Choose a reason for hiding this comment

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

Good, the non-transient field could actually cause (non-Pipeline-related) problems as of 1.483.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are using 1.609.3 so is there any problem or it is ok as I have coded?


public final AbstractBuild<?,?> owner;
public transient Run<?,?> owner;

@Deprecated public transient AbstractBuild<?,?> build;

Expand All @@ -45,11 +48,6 @@ public final class JacocoBuildAction extends CoverageObject<JacocoBuildAction> i
private final String[] inclusions;
private final String[] exclusions;

/**
* Non-null if the coverage has pass/fail rules.
*/
private final Rule rule;

/**
* The thresholds that applied when this build was built.
* TODO: add ability to trend thresholds on the graph
Expand All @@ -58,24 +56,20 @@ public final class JacocoBuildAction extends CoverageObject<JacocoBuildAction> i

/**
*
* @param owner
* @param rule
* @param ratios
* The available coverage ratios in the report. Null is treated
* the same as an empty map.
* @param thresholds
*/
public JacocoBuildAction(AbstractBuild<?,?> owner, Rule rule,
public JacocoBuildAction(
Map<CoverageElement.Type, Coverage> ratios,
JacocoHealthReportThresholds thresholds, BuildListener listener, String[] inclusions, String[] exclusions) {
JacocoHealthReportThresholds thresholds, TaskListener listener, String[] inclusions, String[] exclusions) {
logger = listener.getLogger();
if (ratios == null) {
ratios = Collections.emptyMap();
}
this.inclusions = inclusions;
this.exclusions = exclusions;
this.owner = owner;
this.rule = rule;
this.clazz = getOrCreateRatio(ratios, CoverageElement.Type.CLASS);
this.method = getOrCreateRatio(ratios, CoverageElement.Type.METHOD);
this.line = getOrCreateRatio(ratios, CoverageElement.Type.LINE);
Expand Down Expand Up @@ -202,12 +196,12 @@ public Object getTarget() {
}

@Override
public AbstractBuild<?,?> getBuild() {
public Run<?,?> getBuild() {
Copy link
Member

Choose a reason for hiding this comment

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

This and similar changes are not binary compatible, but I could not find anything in @jenkinsci referring to code in this plugin so probably it is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly I think it is ok in this way.

return owner;
}

public JacocoReportDir getJacocoReport() {
return new JacocoReportDir(owner);
return new JacocoReportDir(owner.getRootDir());
}

/**
Expand Down Expand Up @@ -275,8 +269,8 @@ public Map<Coverage,Boolean> getCoverageRatios(){
/**
* Gets the previous {@link JacocoBuildAction} of the given build.
*/
/*package*/ static JacocoBuildAction getPreviousResult(AbstractBuild<?,?> start) {
AbstractBuild<?,?> b = start;
/*package*/ static JacocoBuildAction getPreviousResult(Run<?,?> start) {
Run<?,?> b = start;
while(true) {
b = b.getPreviousBuild();
if(b==null) {
Expand All @@ -298,12 +292,12 @@ public Map<Coverage,Boolean> getCoverageRatios(){
* @throws IOException
* if failed to parse the file.
*/
public static JacocoBuildAction load(AbstractBuild<?,?> owner, Rule rule, JacocoHealthReportThresholds thresholds, BuildListener listener, JacocoReportDir layout, String[] includes, String[] excludes) throws IOException {
public static JacocoBuildAction load(Run<?,?> owner, JacocoHealthReportThresholds thresholds, TaskListener listener, JacocoReportDir layout, String[] includes, String[] excludes) throws IOException {
//PrintStream logger = listener.getLogger();
Map<CoverageElement.Type,Coverage> ratios = null;

ratios = loadRatios(layout, ratios, includes, excludes);
return new JacocoBuildAction(owner, rule, ratios, thresholds, listener, includes, excludes);
return new JacocoBuildAction(ratios, thresholds, listener, includes, excludes);
}


Expand Down Expand Up @@ -360,4 +354,14 @@ public final PrintStream getLogger() {
// does not run the construct and thus leaves the transient variables empty
return System.out;
}

@Override
public void onAttached(Run<?, ?> run) {
this.owner = run;
Copy link
Member

Choose a reason for hiding this comment

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

Note that this means you could delete the owner parameter from the constructor (assuming it is not used within the dynamic scope of the constructor).

Copy link
Contributor

Choose a reason for hiding this comment

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

@jglick so I can remove the first parameter of this constructor: public JacocoBuildAction(Run<?,?> owner, Map<CoverageElement.Type, Coverage> ratios, ...) without worrying about any side effect. Other plugins currently are not using JaCoCo API.

Copy link
Member

Choose a reason for hiding this comment

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

Right. Note that the load method will return an action whose getBuild() would at that time return null, but once you call someBuild.add(theNewAction) it gets initialized.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

}

@Override
public void onLoad(Run<?, ?> run) {
this.owner = run;
}
}
Loading