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

Ability to specify excludes via file #102

Closed
wants to merge 2 commits into from

Conversation

andrey81inmd
Copy link

I'd like to add the ability to specify jacoco excludes via a file. Currently, it seems only 2 mechanisms exist for excluding code from coverage metrics:

  1. Specify "excludes" parameter in the pom
  2. Add annotations to code you want to exclude

Neither of these really work well to solve the problem we have. We have a large legacy codebase, where unit test standards were not observed. We want to introduce a check in the build for some % of code coverage. In order to do this, we have to exclude hundreds of files from the metrics. We don't want to put that into the pom, and we don't want to modify all these files with annotations.

The solution we came up with basically uses the same mechanism as the existing "excludes" plugin parameter, it just loads the list of files from a file.

@@ -158,6 +159,11 @@ private boolean checkCounter(final CounterEntity entity,
String.format(INSUFFICIENT_COVERAGE, entity.name(),
truncate(ratio), truncate(checkRatio)));
passed = false;
} else {
if (!Double.isNaN(ratio) && !Double.isInfinite(ratio)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have feeling that this piece of code does not relate to the subject of the pull-requestion ("ability to specify excludes via file") and this is completely separate issue.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just added this so I can see what the coverage is when it passes (as of now it only prints the coverage if it fails). If you feel it shouldn't be here, I can remove this.

@marchof
Copy link
Member

marchof commented May 20, 2013

Are we moving in the right direction here?

  • This is a Maven only feature, do we need the same for Ant?
  • It is based on file filtering. What about filtering on class names? This would then also work for JAR files.

Might be related to issues #17 and #15.

@andrey81inmd
Copy link
Author

re: marchof

Does anyone still use Ant? Ok just kidding :) I suppose the same could be done for Ant, but I haven't looked at it. From what I've seen with other tools, maven plugins generally have more features, since more people use them and they're kept up to date.

To answer your second point, this is using the same mechanism as the existing "excludes" parameter. It's just a different way to specify the excludes (I wouldn't want to specify 100 excludes in a pom). If the excludes mechanism is made to work with class names, then this would work with classnames automatically.

I think this is related to issue #15 but it's an addition to that. I don't think this specific feature is discussed in that issue.

@marchof
Copy link
Member

marchof commented May 20, 2013

BTW, unfortunately we already have an inconsistency in the existing includes/excludes parameter, see #34. What is the reasoning behind having an excludes file but no includes file? Shouldn't the API stay symmetrical?

@andrey81inmd
Copy link
Author

I created just excludes mechanism because that's what I needed to solve my problem. But I can definitely add the same for includes. Let me know if you want me to do that, I can work on it when I have some free time.

@LIttleAncientForestKami

Andrey, I'd want that. Though with YAGNI and TDD I see why you've added only excludes. After all, it's seems more likely, right?

So, use case for includes: I want JaCoCo to measure only my code, since I know my colleagues won't write tests anyway (say, for legit reasons). Or I'm writing code that touches legacy and I want to know my own coverage only - thus list of includes is better.

Marchof, somewhat related question. I found nice link on the Wiki - I have a set of generated methods (yeah, METHODS, not classes) that I'd like exclude from coverage - have I missed something in the config options that allows me to do that or that ain't an option ATM with JaCoCo? If it cannot be done at present, how likely do you see it? (feel free to point me to mailing list if you so prefer, I'll comply)

@marchof
Copy link
Member

marchof commented Aug 15, 2013

@LIttleAncientForestKami No, JaCoCo does currently not support filtering on method level. That's why I was asking whether the filtering feature should really be based on input files for the Maven goal only. An alternative would be to filter on class/method names provided by the core APIs.

@LIttleAncientForestKami

@marchof filtering by API is IN the code, so no issues with "how was I supposed to know that we have a file that filters out..." etc. I too think it's better. Especially since it's tool-agnostic. Ant or Maven or Gradle, as long as it's your API rest matters not. How hard would it be to add such annotation - with appropriate processing?

@dsvensson
Copy link

It would be pretty nice to see this for the agentlib as well. Constructing command line parameters is error prone due to shell escaping etc.

@marchof
Copy link
Member

marchof commented Nov 16, 2013

Should be considered in the context of the filtering story. No singular solution for Maven only.

@marchof marchof closed this Nov 16, 2013
@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants