-
Notifications
You must be signed in to change notification settings - Fork 354
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
fix(report-aggregate) Fix report aggregation #625
Conversation
aurelien-baudet
commented
Jun 19, 2019
- [pitest-maven] report-aggregate fails due to test dependencies #621: do not use test dependencies as test source directories
- [pitest-maven] report-aggregate fails while loading mutator class #622: use the name of the mutator (it is not a class name written in mutations.xml)
b50284f
to
67c5a5c
Compare
I would really appreciate if we could have the aggregation fixed |
@hcoles Could you review the PR and merge it if it is OK for you ? Also for the new score calculation, I could add some screenshots and examples to illustrate the interest if you want |
- hcoles#621: do not use test dependencies as test source directories - hcoles#622: use the name of the mutator (it is not a class name written in mutations.xml)
In a real world project with many submodules and particularly a module that has dependencies to other modules, the generated reports can be quite heavy (100MB+). When these reports are parsed using current XML parser, report aggregation fails with GC overhead limit or OutOfMemoryError due to loading of the whole XML data in memory. The new implementation uses StAX in order to avoid loading the whole XML data in memory.
77a5b7e
to
1cf7d55
Compare
@hcoles I have rebased the PR with latest changes to fix report aggregation. Can you merge with master ? |
I also would like to see this fixed. |
@aurelien-baudet Sorry for the silence. If you can resolve the conflicts again and confirm it works, I'll merge this in. |
Sorry but I have currently no time. Moreover, I rebased the code 3 times to resolve conflicts but it had never been merged... |