Closed marchof closed 6 years ago
We would love to have this, specifically the exclusion by comments in the source code. This would allow us to enforce 100% coverage in the builds by default while allowing exceptions when it has been considered, and flagged in the source code. At present we can't enforce anything automatically because there is no suitable way to do this. See https://groups.google.com/forum/#!topic/jacoco/g2l3DHwSyNg for my thread on this.
Just my 2 cents:
I believe that filtering by comments in code (i.e. "Manual Tagging") is a bad practice. Because due to this ability one day you can miss fact that some important part of your code wasn't covered.
And far better to know that you can't achieve 100%, but for example to have 80% as a minimum (i.e. which was already achieved) and 90% as a next target. And in order to enforce such requirements we will provide new Maven Mojo - see #6.
Moreover - in order to really enforce code quality requirements, you might be interesting in a tool like Sonar.
Interesing, my view is that manual tagging was much stricter than percentages because you are saying exactly which lines are not covered, and any new code must be fully accounted for. The tags are visible to the code reviewer so they can decide if they agree. Also when you modify the code later, you know whether your modifications are covered by unit tests. So you never have any uncertainty about what is or isn't covered.
You claim that you can miss un-covered places, but you can easily search through your source for the tags. If you are worried that it doesn't show in the report, then that's something that could be designed in if necessary.
Just out of interest do you have a connection with the Sonar project? It looks really good but I haven't had a chance to look in detail.
Regarding Sonar - yes, I'm one of developers in SonarSource company.
I'd be very interested (for usable Scala reporting) to see this enabled via a rule that filters out all methods that have the same line number as a constructors. e.g.
public scales.utils.IAOne(java.lang.Object);
Signature: (Ljava/lang/Object;)V
LineNumberTable:
line 89: 0
for one of my libraries classes is the constructor, but all of the additional added methods (compiler generated) also share this line number. The actual code (5 methods) have correct line numbers, but that still leaves 298 other methods that could get 0% coverage.
Due to java interop issues the methods unfortunately aren't marked as synthetic.
Needless to say I'm not interested in seeing 0% coverage for methods I never made, nor am I in seeing a lower overall percentage because of this. Right now that makes jacoco unusable for solid reporting, but still very usable for looking at an individual methods coverage of course.
Even a callback of shouldContribute(methodName, className) would allow things to get moving. Fingers crossed.
Concerning the Enum issues (valueOf,...) and other member level exclusion, I've added in the wiki https://github.com/jacoco/jacoco/wiki/FilteringOptions an extension of "excludes" agent's configuration.
Is this Enum issue why the "package com.foo.Enum;" is considered 'uncovered' by JaCoCo?
Yes, the Java compiler created the methods values() and valueOf() for each enum.
I started prototyping a filtering API here: https://github.com/marchof/jacoco-playground-filters
Any updates on this? This blocks our adoption of JaCoCo :(
Your help is greatly appreciated.
Trying to make a huge quality/proper software engineering push over here and some of my devs are a bit devastated that the new way to more cleanly deal with lots of resource (try-with-resources!) is ruining our code coverage. Would love to see this happen!
If you really want to get a 100% coverage, you have to throw an exception in your try block. Mockito and similar tools will help you to accomplish this goal.
I'm not following, how will throwing an exception cover those branches? Which I am to understand, are automatically generated by the compiler and Jacoco has yet to deal with them as it is from a new feature (try-with-resources).
@clee-ancestry Correct. The compiler synthesizes a finally block that closes the resource, and JaCoCo flags this, just like the synthetic Enum methods and friends.
Any progress on this?
There is some experimental work for possible implementation strategies from my side (https://github.com/marchof/jacoco-playground-filters) as well as a JaCoCo fork which already implements some filters (https://github.com/mchr3k/jacoco). Due to time constraints I don't think there will be a filtering solution in JaCoCo within the next months.
@marchof How are those time constraints looking? :-)
I think manual filtering is not a good solution to the various issues closed here. Yes it is perhaps a useful feature overall, but e.g. issue #82 is not solved by this feature alone. There should be no manual work required for the developer! JaCoCo should automatically detect Java compiler generated code and not bother the developers about it. Perhaps you may fix #82 by the way of this filtering, but please do not make users of modern Java and JaCoCo suffer any more than required :) Closing issues like #82 is also not good.
@Macroz As we need a general concept for filtering I want to collect all requirements in this bug. That's the reason why close similar bugs as duplicates.
SBT Scala Filtering +1
It's a long time since this issue is opened. It a really interesting feature. For instance it's implemented in clover and corbetura. There are fork that implement filtering : https://github.com/huangxiwei/jacoco , https://github.com/mchr3k/jacoco since the begining of the year. Why don't you merge those fork into master branch ? Even if all filtering is not implemented at start, main filters needed are listed in the wiki page you have written (Try with resources, sync block, enum static methods). Coverage is a very useful tool, more it's accruate more it's will be usefull. It helps alot when coverage reach a high value, it helps to focus on the right classes.
@acommuni Feel free to use Clover, Cobertura or one of the forks ;-) To get a understanding why I do not merge the forks you see the discussion here: https://github.com/jacoco/jacoco/pull/141#issuecomment-24869447
This topic is almost one year old. Will there be the possiblity to exclude setters and getters in JaCoCo Code Coverage? In Cobertura you can do this with the ignoreTrivial
option and it would be really great to have such an option in JaCoCo too.
I am looking for a way to annotate generated code so that it is excluded from coverage analysis. Annotation of lines and/or methods would help us a lot. This topic seems to address this issue but also does not seem to have materialised in jacoco.
Is there any progress in this area?
Background:
We are using EMF. A lot of the generated code is not actually used. We trust the generator enough to leave all generated code out of our coverage percentage. EMF requires that we adapt the generated code at certain places. We explicitly want to take those manual changes into account for coverage. So we end up with generated classes with manual code in them. All purely generated methods are annotated so an ignoreForCoverage filter can be written. Hooking that up to jacoco would enable us to get far more accurate coverage info.
Hi,
Do we have the filtering option, where I can exclude part of the code in my class example - a method during code coverage analysis. Does the latest release of jacoco has this feature?
I understand the goal to have a clean API. But indeed, at some point, something should happen. I had the same issue with my personal projects and have waited too much time.
I used to use Clover, but it was not very stable from one version to another and isn't open source.
Cobertura was too intrusive so I skipped to JaCoCo which I'm happy with.... apart from the filtering. And the only really important filtering seems to be able to tag the code.
I understand that people can then trick JaCoco by using too much tags. But every new tool can be used for evil. Anyway, that's not my use-case. I'm having 100% coverage on EasyMock. So, I want to know when I'm under that mark and easily find the hole. Without filtering, I can't. I need to dig all around the place to find it.
///COVERAGE:OFF from mchr3k is exactly what I would need.
How can I help? I'm willing to provide a pull request that would be accepted. What would that be?
@henri-tremblay I agree with you that the API is not the highest priority. Highest priority currently are removing annoying compiler artifacts especially for new Java 7 language constructs. As JaCoCo is fully based on class files source file processing (e.g. for comment parsing) is not in scope.
But if the filtering is done by annotations, it won't be really precise. Java 8 will be a bit better since you can put annotations all over the place.
I haven't looked at mchr3k code yet but I would guess the following algorithm:
It doesn't really require a formal parsing, only a grep. It will be slower then when not activated but if not activated by default it won't matter
WDYT?
@henri-tremblay My understanding is that there is nothing to grep. JaCoCo analyzes compiled bytecode, not source files. Thus, there will be no magical ///COVERAGE:OFF to find.
Hum... I understand. It should still be possible from a Maven task. Another solution would be to have a filter file a bit like FindBugs would do. Since the debug info will provide the line numbers, that should be enough.
I understand the generated code is causing issues here, and the ability to tag code as excluded from the coverage, but would you consider splitting the filtering of trivial code out into a separate issue that could be merged quicker? It's incredibly annoying (and an inefficient use of developer time) to have to write unit tests for simple getters and setters. I'd prefer to not exclude the whole class/package since non-trivial setters/getters should still be checked for coverage and we want to be able to pick up when current trivial code becomes non-trivial, which won't happen if it's excluded.
In cobertura you can use IgnoreMethodAnnotation as explained here: https://github.com/cobertura/cobertura/wiki/Ant-Task-Reference#ignore-method-annotation. In lombok you can add configurations to add @SuppressFBWarnings to generated code, which has retention policy of CLASS. Both together is a way to ignore generated methods from test coverage. Does JaCoCo have such a capability?
@marchof
I'm done reading this thread and the relevant links it points to. Since this is about filtering options, I wanted to know whether some filters would be applied systematically (basically all javac-generated false positives) while others (eg. annotation-driven) would actually be optional.
I opened #324 because I don't think asserts should ever be accounted for coverage, but I think that too about many use-cases you have gathered so far.
@henri-tremblay
What you are looking for could probably be done with some sort of post-processing hook instead. Something that could be plugged before the report generation and let you prune coverage metrics you'd consider irrelevant (with the comment stuff for instance). But I merely use jacoco so I really don't know what extension points it offers.
What is the status of this issue ?
Try with resources is several years old. Jacoco still doesn't support a version of Java that we can no longer download from Oracle's website?
@phlogistic JaCoCo supports all class file versions from Java 1.1 to Java 1.8.
By "support" you mean "JaCoCo will run with Java 7 & 8, however if you use language syntax specific to 7 or 8, such as a try with resources block, JaCoCo will incorrectly report that block of code is not tested".
Hoping to see this implemented soon, having an 80% code coverage can hide a lot, while a 99% does not.
As for manual tagging hiding code coverage issues, here's a suggestion; Output the amount of code excluded in the report by yellow, but include it in the overall coverage.
Ia anyone working on this?
I have to agree with @phlogistic on this one. It's not just about try-with-resources -- Enum types are from Java 5 and they still produce uncovered code. I don't think we need a full-fledged configurable filtering solution here; some basic edge case coverage for valueOf
/values
would be a massive step up, then followed by try-with-resources, then any other synthetic code produced by javac.
Hi, I have little clue of how Jacoco counts the coverage, so I have a perhaps dumb question: Why do we care about the generated code, at all? Shouldn't Jacoco count the coverage literally to the original code written by the developer, rather than the "eventual" code generated by the compiler?
@q3769 JaCoCo analyzes bytecode (as generated by compilers), not source code written by a developer. That design choice is fundamental to JaCoCo and cannot be changed, so the only alternative is to add options to filter out things people don't want (note, this can be difficult for exclusions based on control-flow such as assert or try-with-resources). e.g., assume Enum subclasses are generated by javac and assume the coverage of compiler-generated methods is unnecessary (note, these assumptions might not be true).
@bjkail I understand that is the design choice. Starting from the bytecode, does that mean we completely lose what's in the original source code? Why can't it be accounted for retrospectively to the original code?
@q3769 Generally speaking, yes, the original source code is gone, so all that can be done is to use heuristics to guess what was in the original source code. Synthetic methods are the easy and least valuable case. For asserts, analyzing the bytecode to find branches generated by javac for assert statements and matching them to the probes inserted by JaCoCo is difficult. For string-switch, try-finally, or try-with-resources, the bytecode analysis and matching to probes is much harder. I can't speak for the maintainers, but I suspect a pull request would be accepted if someone solves those problems cleanly.
"analyzing the bytecode to find branches generated by javac for assert statements and matching them to the probes inserted by JaCoCo is difficult"
Am I missing something, or are two issues being conflated in this thread? Most of the cases documented at https://github.com/jacoco/jacoco/wiki/FilteringOptions are motivated by the case where JaCoCo gets confused by funny bytecode.
So here we are, talking about auto-generated bytecode, and the occasional inaccuracies it introduces in accounting.
But a number of users are asking a different question: "I have lines of code that I can see in my source file, like assert
s, that JaCoCo is (correctly) telling me have missed branches. I don't care about these missed branches, so I want these missed branches subtracted from the statement coverage report."
Can't filtering of some simple use cases like assert
s be done via some straightforward post-processing of the report? We already know which branches it's claiming are missed, so the choice to not count them should be simple arithmetic. This would satisfy most users.
@SigmaX I'm not a JaCoCo maintainer, so I'll leave further replies to them, but I'll say again that deriving that kind of information from source code is contrary to the design of JaCoCo and therefore cannot be reliably implemented. If your users are demanding this feature and you think you can satisfy them by post-processing, then you should do it, but I don't think that's the right approach for the JaCoCo project.
The main goal of code coverage is to identify where an effort is needed in order to fulfill code quality requierments. If the tool raises false positive "not coverage part", you cannot mastered your source code. For instance having an issue on Enum classes showing that the "valueOf" method is not covered is not relevant beacause if you do not need this "auto generated" method you do not have to test it. The goal of doing coverage on java code is not to to cover bytecode but the real java source code written by developpers. Moreover there are part of code that you know you cannot reach with unit code tests, for instance error code branches that are written in order to protect the code but is not possible to simulate the cause of entering this part of code. At the end of a developpement cycle the code coverage should be at a level of 100%, it should be reached by covering code with unit tests and ignoring part of code that you know you cannot reach them or you do not want to reach them. Then if new code are written you can check if it intoduce new "not covered part". In fact the code coverage reports should contain code/branch covered, code/branch not covered and code/branch considered as covered (I mean if it's filtered by a manual mark it's OK). If code coverage reports are not reliable because it show wrong issue (Enum, try catch, synchronized block, ...) you cannot control the code. The final goal of doing code coverage is not the use of coverage tool but the control your code. It's very frustrating that Jacoco is a very beautiful tool but do not fulfill real life concerns about code qualiity. Mchr3k fork is doing part of the job but it's not up to date.
What if, after the bytecode coverage is counted as-is right now, we read in the original source code, and compare and "correct" the bytecode coverage number?
Is that hard? If so, is there any other coverage tool that only counts the original source code coverage - the coverage of the generated code is really not that interesting.
@q3769 There is, Clover by Atlassian works with source code instrumentation. This requires deep integration into your build cycle. Focus of JaCoCo is providing a light weight and non-intrusive tool.
Wow, this conversation deviated. I just want to ignore getters/setters. Devs here are literally spending a time creating tests for this (which do nothing - I don't blame them). Why has this even been open for two years? Time to see the forest for the trees.
JaCoCo should support configurable filtering options for code coverage analysis. This page lists of use cases and requirements: https://github.com/jacoco/jacoco/wiki/FilteringOptions