-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Jacoco Fixes #16666
Jacoco Fixes #16666
Conversation
LocalProject project = LocalProject.loadWorkspace(targetdir.toPath()); | ||
runtimeDeps.add(project.getKey()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aloubyansky do you know if the gradle code below would also need a similar change? I don't have any multi module gradle projects locally to test on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't tried it but looking at the change, it makes sense. BTW, the AppModel already includes getLocalProjectArtifacts()
method, it could simplify your loops here.
I've tried this commit (cherry-picked onto the Quarkus 1.13 branch, 3b461a2) but the warnings + stacktraces are still logged. |
Sorry, was too fast without reading the explicitly mentioned docs change. After disabling the jacoco-plugin in that module, the warnings and stacktraces are no longer logged. The reason why projectnessie has this jacoco setup is to collect the code-coverage for the source in the Maven module that uses Quarkus and the code-coverage for other classes. I can't really test the whole CI spiel locally, so cannot confirm whether all that works. Would be great to get this PR backported into the 1.13 branch. |
@@ -175,6 +175,9 @@ Now we need to add Jacoco to our project. To do this we need to add the followin | |||
This Quarkus extension takes care of everything that would usually be done via the Jacoco maven plugin, so no additional | |||
config is required. | |||
|
|||
WARNING: Do not use both the extension and the plugin, if you configure both you will get lots of errors about classes | |||
already being instrumented. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't that contradict paragraph "7. Coverage for tests not using @QuarkusTest"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartwdouglas could you have a look at this comment?
This workflow status is outdated as a new workflow run has been triggered. Failing Jobs - Building 5f520e3
|
- Note in docs that you can't use both plugin and extension - Make sure only actual dependencies are taken into account - If an error occurs print out an error report
In 1.13.2 and prior I can use the Jacoco extension in a multi-module project and it includes the coverage of all the modules in the project. Now as of 1.13.3 this is no longer the case and it only includes the direct module in which the @QuarkusTest is found. Is there a way to get the plugin to analyse the coverage of all the modules? |
Can you file an issue? That should work. |
Created a reproducer: #17086 |
Fixes #16358