support jacoco aggregate

[7sean68]

for multimodule maven projects running mvn jacoco:report-aggregate generates a coverage reports of dependency modules. but coverage gutters doesn't show coverage in dependency module.

I checked the generated xml and found that the difference a<group name="dependency-module-name"></group> around packages' tags. removing this tag makes coverage gutters display coverage details correctly for dependency module. so I think it can be easily fixed.

[ryanluker]

@7sean68 Thanks for the ticket! Most likely, we will need to fix this in the jacoco upstream dependency 🤔. https://www.npmjs.com/package/jacoco-parse

It seems like the project is not maintained anymore though and someone will have to look for a more up-to-date fork or do the fix in a fork themselves.

[7sean68]

I searched but couldn't find a suitable fork ... I can create a fork and implement this in it then publish it on npm. will you be willing to use it?

[7sean68]

I forked it to https://www.npmjs.com/package/@7sean68/jacoco-parse

and added support for group tag keeping all backward compatibility. also I fixed some vulnerabilities including 1 critical.

[ryanluker]

I forked it to https://www.npmjs.com/package/@7sean68/jacoco-parse

and added support for group tag keeping all backward compatibility. also I fixed some vulnerabilities including 1 critical.

Looks great, thanks for taking this on! If you can setup a PR against the extension, I will do a review of the new forked dependency and we can see if all the tests pass in the CI workflow.

[ryanluker]

@7sean68 Only initial minor issue I see (after looking through https://github.com/vokal/jacoco-parse/compare/master...7sean68:jacoco-parse:master#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R21), is that the repo has mocha as a dependency rather than a dev one? (this isn't something you did with the fork and I am not sure the lift to move that into a dev dependency instead?)

Nice job with the fork and the extra test coverage though! 👍🏻

[7sean68]

hmm... I didn't use mocha before ... but if it's just a testing library ... I can move it to devDependencies or remove it altogether as I added jest. I'll have to fix scripts also.

BTW the old version mocha was the one causing the critical vulnerability. so, I had to do this major upgrade.

[ryanluker]

hmm... I didn't use mocha before ... but if it's just a testing library ... I can move it to devDependencies or remove it altogether as I added jest. I'll have to fix scripts also.

BTW the old version mocha was the one causing the critical vulnerability. so, I had to do this major upgrade.

Yeah old mocha has security complaints in this repo as well but within the example project folders 😅. Ping me once the PR is up to swap out the dependency with yours and I can get the CI workflow running against it for yeah.

[7sean68]

@ryanluker can you check this PR #405 ?

[ryanluker]

@ryanluker can you check this PR #405 ?

Sure thing!

---

Took a first look and left one comment, otherwise looking great to me (especially the added example + test coverage! thanks a bunch for htis).

[ryanluker]

@7sean68 Thanks again for helping maintain and enhance the extension! This will go out in the 2.11.0 release (shouldn't be more than a couple weeks as I will descope some of the items to allow for this to go out quicker).