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

Filtering options for coverage analysis #15

Closed
marchof opened this issue Aug 29, 2012 · 110 comments
Closed

Filtering options for coverage analysis #15

marchof opened this issue Aug 29, 2012 · 110 comments

Comments

@marchof
Copy link
Member

@marchof marchof commented Aug 29, 2012

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

@ghost
Copy link

@ghost ghost commented Sep 18, 2012

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.

@Godin
Copy link
Member

@Godin Godin commented Sep 18, 2012

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.

@ghost
Copy link

@ghost ghost commented Sep 18, 2012

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.

@Godin
Copy link
Member

@Godin Godin commented Sep 18, 2012

Regarding Sonar - yes, I'm one of developers in SonarSource company.

@chris-twiner
Copy link

@chris-twiner chris-twiner commented Sep 19, 2012

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.

@fdaugan
Copy link

@fdaugan fdaugan commented Nov 7, 2012

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.

@tgoldenatwork
Copy link

@tgoldenatwork tgoldenatwork commented Nov 7, 2012

Is this Enum issue why the "package com.foo.Enum;" is considered 'uncovered' by JaCoCo?

@marchof
Copy link
Member Author

@marchof marchof commented Nov 7, 2012

Yes, the Java compiler created the methods values() and valueOf() for each enum.

@marchof
Copy link
Member Author

@marchof marchof commented Feb 14, 2013

I started prototyping a filtering API here: https://github.com/marchof/jacoco-playground-filters

@joracine
Copy link

@joracine joracine commented May 7, 2013

Any updates on this? This blocks our adoption of JaCoCo :(

Your help is greatly appreciated.

@clee-ancestry
Copy link

@clee-ancestry clee-ancestry commented May 22, 2013

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!

@fdaugan
Copy link

@fdaugan fdaugan commented May 22, 2013

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.

@clee-ancestry
Copy link

@clee-ancestry clee-ancestry commented May 22, 2013

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).

@TomRK1089
Copy link

@TomRK1089 TomRK1089 commented May 22, 2013

@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.

@laeubi
Copy link

@laeubi laeubi commented Aug 27, 2013

Any progress on this?

@marchof
Copy link
Member Author

@marchof marchof commented Aug 28, 2013

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.

@huntc
Copy link

@huntc huntc commented Nov 17, 2013

@marchof How are those time constraints looking? :-)

@Macroz
Copy link

@Macroz Macroz commented Dec 3, 2013

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.

@marchof
Copy link
Member Author

@marchof marchof commented Dec 4, 2013

@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.

@vincenzovitale
Copy link

@vincenzovitale vincenzovitale commented Jan 25, 2017

Hi @marchof, any news on this?
In TomTom we are using Lombok, Jacoco and SonarQube and having this would help us a lot :)

Thanks a lot in advance,
Vincenzo.

@marchof
Copy link
Member Author

@marchof marchof commented Jan 26, 2017

@Gaibhne This feature request is in status open. The wiki pages say:

This page discusses a not yet available feature!

What exactly do you not understand here?

@marchof
Copy link
Member Author

@marchof marchof commented Jan 26, 2017

@vincenzovitale Nice to hear that TomTom is using JaCoCo! Unfortunately nobody is currently working on this topic. I still hope that one day we're blessed with resources and creativity to implement a powerful filtering mechanism. Cheers, -marc

@Godin
Copy link
Member

@Godin Godin commented Jan 26, 2017

@Gaibhne sad to hear about your loss of time, but

Why this ticket exists and open: because we use issue tracker to track issues which we'd like to resolve sometime and to show that there is work for potential contributors. Tickets that will be resolved really soon are associated with exact next version explicitly. Btw exactly "open" not "in-progress".

Why wiki exists: because we use it to persist information that needs to be shared between developers, we are really far from each other - located in different countries. Moreover we believe in freedom of information and want to make sure that all discussions and insights about JaCoCo are available to the public. And moreover - we recently updated wikis to better describe bytecode that requires filtering.

We don't have important hidden discussions - even our dev mailing list can be read by anyone: https://groups.google.com/forum/#!forum/jacoco-dev

Official documentation about features that will be available in next version are on our site - http://www.jacoco.org/jacoco/trunk/doc/changes.html So please consider reading official documentation prior to googling.

@Gaibhne
Copy link

@Gaibhne Gaibhne commented Jan 26, 2017

@Godin Thank you for the extensive answer. I'm sorry about my earlier posts and have deleted them. I had seen the Wiki a while ago and the other day I decided to see if there was a public test ready, so I was very disappointed when it turned out I was wrong, but that frustration should not have been a reason for my unprofessional venting. My apologies, and thank you for your work on Jacoco.

@cuioss
Copy link

@cuioss cuioss commented Feb 24, 2017

Update on the lombok issue:
With lombok v1.16.14 the generated code can be annotated at byte-code level with @lombok.Generated, see projectlombok/lombok#1014
The missing link is now the ability to exclude methods identified by a configured annotation, simliar to cobertura: (sample by @Gaibhne )

<ignoreMethodAnnotations>
   <ignoreMethodAnnotation>lombok.Generated</ignoreMethodAnnotation>
</ignoreMethodAnnotations>

@t1
Copy link

@t1 t1 commented Mar 9, 2017

Is anybody working on this already? I took a look and found a place where I'd put it. I assume the main work would be to transport the config to the agent, but that looks straight forward, too. I quite like the code, actually :-)

And is it okay to discuss this here?

@marchof
Copy link
Member Author

@marchof marchof commented Mar 9, 2017

@t1 Except anylysis of the cases (see wiki) we're not working on this. Also

I assume the main work would be to transport the config to the agent

Actually filtering should happen at analysis time. Therefore the agent does not need any parameters for filtering.

From my point of view the main work is to efficiently match bytecode patterns and design an API that allows to hook filters into the analysis process. Any clever idea is welcome here!

@t1
Copy link

@t1 t1 commented Mar 9, 2017

@marchof: Thanks for the fast feedback. The FilteringOptions have a different scope, I guess. I want to add a configuration to the maven plugin to set the annotations that make the ClassInstrumenter#visitMethod skip instrumentation. Following Coverturas lead, I'd use something like this in the pom:

<configuration>
  <instrumentation>
    <ignoreMethodAnnotations>
      <ignoreMethodAnnotation>lombok.Generated</ignoreMethodAnnotation>
    </ignoreMethodAnnotations>
  </instrumentation>
</configuration>

Lombok is the use case I have at hand, but there are other code generators using different annotations, so this configuration would have to be moved to the agent, wouldn't it?

I'll take a look, if I can make it work.

@marchof
Copy link
Member Author

@marchof marchof commented Mar 9, 2017

@t1 The agent always instruments all methods. Whether a method is included in the coverage report is a plain decision at analysis time. If we would adjust instrumentation in addition, we would need to always configure both places (agent and analysis) always the exact way. Note: Exec files do not come with any meta information about the probes. Probes are derived from the structure of the class file only. Therefore this process must create the same results at instrumentation and analysis time. See section "Why can't JaCoCo simply use the class name to identify classes?" here http://www.jacoco.org/jacoco/trunk/doc/classids.html

@t1
Copy link

@t1 t1 commented Mar 10, 2017

@marchof: Oh, I was on the wrong track. Thanks for the hint.

I've reconsidered what you where saying and it's probably more according to the JaCoCo philosophy that JaCoCo should be smart, and not every developer be forced to know and consider and configure things like this. So it's probably even better to hard code the lombok.Generated annotation.

@davidburkhart
Copy link

@davidburkhart davidburkhart commented Sep 27, 2017

Are there any plans on implementing and releasing the filtering feature? Last comment is 6 months old...

@marchof
Copy link
Member Author

@marchof marchof commented Sep 27, 2017

@davidburkhart We're actively working on filters. See our change log what is already available on snapshot builds: http://www.jacoco.org/jacoco/trunk/doc/changes.html
We might be able to do a release this year.

@marchof
Copy link
Member Author

@marchof marchof commented Sep 27, 2017

To not confuse people I'm closing this issue as we're using separate PRs for specific filtering scenarios now.

@marchof marchof closed this as completed Sep 27, 2017
@abinet
Copy link

@abinet abinet commented Oct 19, 2017

What about ignoring main methods for SpringBootApplication?

For this class I get 0% coverage.

@SpringBootApplication
public class MyApplication {

	public static void main(String[] args) {
		SpringApplication.run(MyApplication.class, args);
	}
}

@t1
Copy link

@t1 t1 commented Oct 19, 2017

@abinet: I'm not sure if I get your question right: You have a SpringBootApplication that you do test, but not the main method itself? I guess that you could write some extra logic in that main method, even if it's not idiomatic. It's not generated code, after all, so you should write a test covering this. Or even better, add a feature to Lombok to generate the main method! That would solve not only the coverage problem, but also remove that boiler plate code ;-)

@abinet
Copy link

@abinet abinet commented Oct 19, 2017

@t1 Thanks for the answer. You got the point right but it looks too dogmatic for me.

@sshcherbakov
Copy link

@sshcherbakov sshcherbakov commented Oct 19, 2017

@t1 that actually means, that to get 100% test coverage, we must run the full application necessarily starting from the main method, even when its complete code (except one liner starter that only calls third party Spring Boot code) is 100% test covered.
It would not be a problem in this particular case of a single liner that Spring Boot framework rightfully requires, if we could exclude that method from code coverage check. But it seems that it is not possible to do either.
So we are forced to spend significant effort on testing Spring Boot Framework instead of concentrating on the application code.

@t1
Copy link

@t1 t1 commented Oct 19, 2017

I don't think that forcing 100% coverage is generally worth the effort. I'd just live with 99% in this case ;-)

But I guess you could just as well simply cheat by manually annotating the main method as @lombok.Generated.

Just to make my point clear: I don't think that JaCoCo should assume that there never will be any logic in a Spring-Boot main method; just as it should not assume that there never will be any logic in a getter. These things are real code written by hand, so they can contain bugs, and they should be covered. Only when things are generated (synthetic code, lombok generated, etc.), then JaCoCo should not count them as uncovered; we can assume that the tools generating the code are okay.

@SchulteMarkus
Copy link

@SchulteMarkus SchulteMarkus commented Oct 19, 2017

@abinet
In my opinion, the execution of this main()-method should be done in the integration-test-phase of your Maven-build. I am not familiar with Gradle, but I hope, they got something similar.
You could bind your jacoco-report not to end after test-phase, but after integration-test-phase (IMO bad pattern), this way the execution of your main()-method will be recorded and part of your JaCoCo-report.

A little bit off-topic...
If you want to integration-test your Spring-application, build as a docker-image, maybe you want to have a look at my repo SchulteMarkus/docker-compose-rule-spark-demo. In this repo, Java-spark is used, not Spring, but it should be easy to adapt (as this testing is about a jar-file, in this case from Java-spark, in your case form Spring). For testing, HTMLUnit is used.

@Godin Godin moved this from To Do to Awaiting triage in Filtering Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Filtering
  
Awaiting triage
Development

No branches or pull requests