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

[Gradle Task] JaCoCo compatible with gradle 5.x #11321

Conversation

mickael-caro-sonarsource
Copy link
Contributor

No description provided.

@mickael-caro-sonarsource mickael-caro-sonarsource changed the title JaCoCo code coverage compatible with gradle 5.x [Gradle Task] JaCoCo compatible with gradle 5.x Sep 11, 2019
Copy link
Contributor

@madhurig madhurig left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. Is there any issue with compatibility of the latest jacoco version with older versions of gradle?

Tasks/ANTV1/task.json Outdated Show resolved Hide resolved
Tasks/MavenV3/task.json Outdated Show resolved Hide resolved
@madhurig
Copy link
Contributor

@mickael-caro-sonarsource - thank you for your contribution ✨!

@mickael-caro-sonarsource
Copy link
Contributor Author

Thanks for the review. Given this changelog : https://www.jacoco.org/jacoco/trunk/doc/changes.html No depreciations are mentionned, so it should be ok. I will bump the versions correctly now.

@mickael-caro-sonarsource
Copy link
Contributor Author

@madhurig after checking the documentation, some Gradle versions might not work anymore with this change. Which versions are you officially supporting within those tasks ? Thanks.

@pjquirk
Copy link
Member

pjquirk commented Sep 11, 2019

after checking the documentation, some Gradle versions might not work anymore with this change. Which versions are you officially supporting within those tasks ? Thanks.

@mickael-caro-sonarsource I don't see any documentation for officially supported versions, unfortunately (looking here). What incompatibilities are you seeing, and which versions may not work because of them? I went back as far as Gradle 3.0 (released Aug 2016) and saw the file() method still available.

@mickael-caro-sonarsource
Copy link
Contributor Author

I have a doubt, considering this chapter, on which the syntax is the current one : https://docs.gradle.org/3.0/userguide/jacoco_plugin.html#sec:jacoco_report_configuration

@@ -26,9 +26,9 @@ subprojects {

reports {
html.enabled = true
html.destination "\${buildDir}/jacocoHtml"
html.destination file("\${buildDir}/jacocoHtml")
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the $ is being escaped here, so the end result would be path/to/build.gradle/${buildDir}/jacocoHtml, rather than replacing ${buildDir} with it's value. Or am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intended to be replace in the build.gradle file instead, as a project property.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see now, phew that's confusing

@pjquirk
Copy link
Member

pjquirk commented Sep 11, 2019

That is just Groovy syntax, $var or ${var} both work.

@pjquirk pjquirk merged commit aacf998 into microsoft:master Sep 11, 2019
@pjquirk
Copy link
Member

pjquirk commented Sep 11, 2019

Thanks @mickael-caro-sonarsource!

@adriaanthomas
Copy link

This would also help the MavenV3 task, as JaCoCo coverage doesn't currently work with Java 11 code there. Is there any idea when this will be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants