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

centic9 · 2016-01-20T21:34:19Z

Continued from PR #63 without all the whitespace changes...

…related whitesace/formatting changes

jenkinsadmin · 2016-01-21T01:02:21Z

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

andresrc · 2016-01-21T11:02:12Z

👎 not keeping the original authorship. Not polite and discourages contribution.

lordofthejars · 2016-01-26T09:01:32Z

oh please, I would like to have this issue fixed and released by Thursday, anyone with permissions think it is going to be possible?

portenez · 2016-02-02T20:21:59Z

Is there any chance this could be pulled/merged in the near future?

@andresrc, I think @lordofthejars is more concerned with getting this released than with the authorship, and I could also use this fix. Also, there's the link to the original PR, which will show the original authorship. (@lordofthejars)

In any case, thanks all.

lordofthejars · 2016-02-02T21:11:30Z

Exactly, I don t care the authorship I want that people can improve how
they are working.
El 2/2/2016 9:22 p. m., "Victor Garcia" [email protected] escribió:

Is there any chance this could be pulled/merged in the near future?

@andresrc https://github.com/andresrc, I think @lordofthejars
https://github.com/lordofthejars is more concerned with getting this
released than with the authorship, and I could also use this fix. Also,
there's the link to the original PR, which will show the original
authorship. (@lordofthejars https://github.com/lordofthejars)

In any case, thanks all.

—
Reply to this email directly or view it on GitHub
#66 (comment)
.

jglick · 2016-02-04T21:17:12Z

src/main/java/hudson/plugins/jacoco/JacocoBuildAction.java

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


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).

@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.

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.

jglick · 2016-02-04T21:22:02Z

Other than the incorrect use of checkRoles, looks basically good. Seems it need a remerge with master though.

lordofthejars · 2016-02-05T12:25:04Z

@centic9 are you going to continue with this issue or you prefer I get it again and apply the changes suggested by @jglick

centic9 · 2016-02-05T13:40:26Z

Just go ahead please, I actually lack the insight into the Jenkins internals necessary here anyway, just let me know when it is ready to merge

removes unused code

lordofthejars · 2016-02-05T16:42:16Z

@centic9 @jglick The changes you mention has been merged to this PR. Do we need bees or are not necessary since the PR was not originated by me. Of course a review about I have done the cahnges correctly would be much appreciated.

jglick · 2016-02-05T16:50:16Z

pom.xml

@@ -92,6 +92,7 @@ THE SOFTWARE.
      <project.build.outputEncoding>UTF-8</project.build.outputEncoding>
      <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
      <jacoco.version>0.7.5.201505241946</jacoco.version>
+        <workflow.version>1.12</workflow.version>


Why is this here? Seems unused. (Of course you would use it if you wanted to add automated tests.)

fix FreeStyle projects to work as expected

lordofthejars · 2016-02-09T13:36:49Z

@jglick ok I think that now it is ready to be merged into master. If you agree let's put a bee and I talk with James for the same.

lordofthejars · 2016-02-17T08:11:19Z

@reviewbybees

ghost · 2016-02-17T08:17:15Z

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

Conflicts: src/main/java/hudson/plugins/jacoco/JacocoPublisher.java

jmvanryn · 2016-03-23T17:51:59Z

Is there any update on this? @reviewbybees ?

lordofthejars · 2016-03-23T18:14:54Z

For me it is ok, but after pushing hard for trying to release this and after the time I decided to give it up.

isubuz · 2016-04-04T09:34:06Z

Are there any plans to merge this soon?

fixes code review issues

centic9 · 2016-04-15T17:11:57Z

Sure, just let me know when you think it is ready for merge so we can finally release this.

faheem-nadeem · 2016-04-25T11:34:40Z

Hi guys any ETA on this... we are also migrating to pipeline plugin, and it seems this essential plugin is still missing :(

lordofthejars · 2016-04-25T11:38:14Z

@jglick must review the last changes.

ben-gineer · 2016-05-07T02:37:07Z

I'm also moving to the Pipelne plugin and this PR would be a big help.i

centic9 · 2016-05-08T15:03:07Z

In order to allow others to give feedback more easily I have published a pre-release with these changes at https://github.com/jenkinsci/jacoco-plugin/releases/tag/2.1.0-beta1, please give it a try by manually installing the .hpi file in the Jenkins installation via "manage jenkins - plugins - advanced options - upload plugin" and report here if it works for you or not.

centic9 · 2016-05-08T18:19:44Z

BTW, a local build fails for me with the following on 1.638, seems the onAttached() is not called there? The changes set the required jenkins version to 1.609.3.

ERROR: Build step failed with exception
java.lang.NullPointerException
at hudson.plugins.jacoco.JacocoBuildAction.getJacocoReport(JacocoBuildAction.java:204)
at hudson.plugins.jacoco.JacocoBuildAction.getResult(JacocoBuildAction.java:219)
at hudson.plugins.jacoco.JacocoPublisher.perform(JacocoPublisher.java:490)
at hudson.tasks.BuildStepCompatibilityLayer.perform(BuildStepCompatibilityLayer.java:76)
at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:782)
at hudson.model.AbstractBuild$AbstractBuildExecution.performAllBuildSteps(AbstractBuild.java:723)
at hudson.model.Build$BuildExecution.post2(Build.java:185)
at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:668)
at hudson.model.Run.execute(Run.java:1763)
at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
at hudson.model.ResourceController.execute(ResourceController.java:98)
at hudson.model.Executor.run(Executor.java:410)

leifbladt · 2016-05-09T12:44:15Z

I can confirm the mentioned NullPointerException with Jenkins 2.2

mheinzerling · 2016-05-30T08:01:50Z

Created #73 to fix the NPE.

centic9 · 2016-05-30T21:09:59Z

After the NPE was fixed via #73 I have now merged these changes onto trunk. Please test it some more by running "mvn package" and then importing the resulting target/jacoco.hpi manually in Jenkins -> Manage -> Plugins -> Advanced.

anyone-can-test · 2016-06-02T11:30:20Z

I tested the latest jacoco plugin (mvn package in my local pc) and the pipeline functionality.
The coverage result was good.
But, there was no Code Coverage Trend in the main screen of a job.

mheinzerling · 2016-06-02T13:21:47Z

Might be related to JENKINS-31202.

mscharp · 2016-07-15T16:25:23Z

Could someone post, or link to a post that shows, what the syntax would be in order to use the JacocoPublisher?

mheinzerling · 2016-07-18T03:55:08Z

Just run the current master or even mheinzerling_2.1.0 with a current jenkins and you will get the pipeline configuration util for free. You can configure your step with the usual graphical interface.

mpchlets · 2016-07-29T21:43:28Z

@mheinzerling - when using pipeline from a Jenkinsfile, you cannot use the graphical interface to configure.

mheinzerling · 2016-07-29T22:36:28Z

@mpchlets, more details please. I was talking about the pipeline snippet page.

jreimone · 2016-09-26T07:04:38Z

How to use the JacocoPublisher in a pipeline script? I cannot find any syntactical descriptions. I just tried to add step([$class: 'JaCoCoPublisher']) into my script but I get java.lang.UnsupportedOperationException: no known implementation of interface jenkins.tasks.SimpleBuildStep is named JaCoCoPublisher with JaCoCo plugin v.2.0.1

thatsIch · 2016-09-26T10:05:23Z

@jreimone This is not supported in 2.0.1. Update at least to 2.1.0-beta

jreimone · 2016-09-26T11:52:13Z

@thatsIch, thank you very much. With 2.1.0-beta release I got an NPE but it worked successfully with the latest master I Built locally. Is there a possibility to display the trend of the coverage in the job's overview, as it is also done by the JUnit test results?

thatsIch · 2016-09-30T20:07:53Z

You can use the Jacocopublisher to do that, this allows to display it in each build (in #1, #2 etc on the left side). Sadly the overview on the dashboard is not working since some integration is not working.

tadgh · 2016-09-30T22:43:34Z

Would really like the ability to have it in dashboard, but in build is good enough for now! I'm in the same place as Jreimone

thatsIch · 2016-10-01T18:06:19Z

You can also use the html publisher to publish the index.html of JaCoCo and give you a quick link to the latest one. Every solution sadly sub-optimal, but better than every dev stalling his IDE waiting for his coverage

vladdu · 2016-12-16T09:54:39Z

I just tried step([$class: 'JaCoCoPublisher']) with 2.1.0 and I get "no known implementation of interface jenkins.tasks.SimpleBuildStep is named JaCoCoPublisher"... Should it work?

thatsIch · 2016-12-16T18:05:49Z

It is called JacocoPublisher

vladdu · 2016-12-16T18:33:26Z

Oh, silly me, I just copied from a comment above and didn't check... Thanks!

thatsIch · 2016-12-16T21:38:34Z

Always look into the source :P it is actually easier than to rely on hear-say. That is how I know about the class name.

You are welcome :)

Manually merge changes from PR63 as the actual PR contains lots of un…

c81f231

…related whitesace/formatting changes

centic9 mentioned this pull request Jan 20, 2016

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

Closed

jglick reviewed Feb 4, 2016
View reviewed changes

Merge remote-tracking branch 'origin/master' into pr63_manualmerge

a41a986

lordofthejars and others added 3 commits February 5, 2016 16:12

removes unused code

0810660

fixes wrong tests.

4e4fb1b

Merge pull request #69 from lordofthejars/pr63_manualmerge

28a7f26

removes unused code

jglick reviewed Feb 5, 2016
View reviewed changes

lordofthejars and others added 2 commits February 5, 2016 18:05

fix FreeStyle projects to work as expected

ebdd68d

Merge pull request #70 from lordofthejars/pr63_manualmerge

7ff656f

fix FreeStyle projects to work as expected

Merge branch 'master' into pr63_manualmerge

b64da06

Conflicts: src/main/java/hudson/plugins/jacoco/JacocoPublisher.java

centic9 mentioned this pull request Mar 31, 2016

Use full precision when comparing coverage to thresholds #71

Merged

lordofthejars and others added 2 commits April 15, 2016 18:36

fixes code review issues

249bd6d

Merge pull request #72 from lordofthejars/pr63_manualmerge

adc858d

fixes code review issues

centic9 merged commit adc858d into master May 30, 2016

centic9 deleted the pr63_manualmerge branch May 30, 2016 21:10

seanf mentioned this pull request Mar 3, 2017

Fix step name for JacocoPublisher jenkinsci/pipeline-plugin#432

Merged

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

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

Conversation

centic9 commented Jan 20, 2016

jenkinsadmin commented Jan 21, 2016

andresrc commented Jan 21, 2016

lordofthejars commented Jan 26, 2016

portenez commented Feb 2, 2016

lordofthejars commented Feb 2, 2016

jglick Feb 4, 2016

Choose a reason for hiding this comment

lordofthejars Feb 5, 2016

Choose a reason for hiding this comment

jglick Feb 5, 2016

Choose a reason for hiding this comment

lordofthejars Apr 15, 2016

Choose a reason for hiding this comment

jglick commented Feb 4, 2016

lordofthejars commented Feb 5, 2016

centic9 commented Feb 5, 2016

lordofthejars commented Feb 5, 2016

jglick Feb 5, 2016

Choose a reason for hiding this comment

lordofthejars commented Feb 9, 2016

lordofthejars commented Feb 17, 2016

ghost commented Feb 17, 2016

jmvanryn commented Mar 23, 2016

lordofthejars commented Mar 23, 2016

isubuz commented Apr 4, 2016

centic9 commented Apr 15, 2016

faheem-nadeem commented Apr 25, 2016 • edited Loading

lordofthejars commented Apr 25, 2016

ben-gineer commented May 7, 2016

centic9 commented May 8, 2016

centic9 commented May 8, 2016

leifbladt commented May 9, 2016

mheinzerling commented May 30, 2016 • edited Loading

centic9 commented May 30, 2016

anyone-can-test commented Jun 2, 2016

mheinzerling commented Jun 2, 2016 • edited Loading

mscharp commented Jul 15, 2016

mheinzerling commented Jul 18, 2016

mpchlets commented Jul 29, 2016

mheinzerling commented Jul 29, 2016 • edited Loading

jreimone commented Sep 26, 2016 • edited Loading

thatsIch commented Sep 26, 2016

jreimone commented Sep 26, 2016

thatsIch commented Sep 30, 2016

tadgh commented Sep 30, 2016

thatsIch commented Oct 1, 2016

vladdu commented Dec 16, 2016

thatsIch commented Dec 16, 2016

vladdu commented Dec 16, 2016

thatsIch commented Dec 16, 2016

faheem-nadeem commented Apr 25, 2016 •

edited

Loading

mheinzerling commented May 30, 2016 •

edited

Loading

mheinzerling commented Jun 2, 2016 •

edited

Loading

mheinzerling commented Jul 29, 2016 •

edited

Loading

jreimone commented Sep 26, 2016 •

edited

Loading