`val logger = KotlinLogging.logger {}` reports no code coverage about kotlin-logging HOT 21 CLOSED

Added it here: https://github.com/MicroUtils/kotlin-logging/wiki/Coverage-reporting

from kotlin-logging.

Good morning everyone,

yesterday I also stumpled upon JaCoCo marking the line

private val logger = KotlinLogging.logger {}

as untested. Adding the exclusion like in https://github.com/oshai/kotlin-logging/wiki/Coverage-reporting did not work, because the logger seems to be compiled into a static field in class MyClassKt when defined as a top-level function as suggested on https://github.com/oshai/kotlin-logging/wiki. What helped a bit was to move the logger into a companion object

companion object: KLogging()

However, we have some classes where we want to use a KotlinLogger which extend classes that alredy define a logger (e.g., org.springframework.web.filter.GenericFilterBean, where the logger in the companion object is then shadowed by the logger of the superclass. Due to this problem, we uniformly changed the logger name to kLogger in our project, but this doesn't work anymore with the KLogging class.

Maybe defining an own class similar to KLogging with a different property name would help, but I would rather like to avoid adding a companion object just for logging. Another idea would be to use

private val logger = KotlinLogging.logger {"package.Class"}

but has other downsides, of course.

Does anybody has any other ideas?

from kotlin-logging.

I don't know what coverage tool you're using, but when I tested it in IntelliJ coverage it report as the line was visited (see the green mark on the side bar):

from kotlin-logging.

JaCoCo reports the line as visited, but missing a branch. IntelliJ doesn't report properly at all. It misses half the stuff that jacoco covers.

from kotlin-logging.

We can add method invocation which might solve the issue, but I'm hesitant to do that just for coverage since it is a waste of cpu cycles. Is adding it as a configuration a good alternative?
Another option is to use the inheritance flavour.
What do you think?

from kotlin-logging.

I'm not sure what you mean by

adding it as a configuration

And for the inheritance option, do you mean inherit a logging class?

from kotlin-logging.

adding it as a configuration will be something like starting your coverage run with a system property like -Dkotlin.logging.coverage

For inheritance I mean inherit KLogging as you can see in the example here: https://github.com/MicroUtils/kotlin-logging/wiki#usage

from kotlin-logging.

@snowe2010 will you be able to test such flag or should I close the issue?

from kotlin-logging.

Sorry, I've been out of the country for a while. I will try to get back to this tonight!

from kotlin-logging.

So the companion object version works just fine. Thank you very much. How does that actually work though? What is placing a logger object into my object from the Companion? And I am still not sure I understand what you mean by

adding it as a configuration will be something like starting your coverage run with a system property like -Dkotlin.logging.coverage

from kotlin-logging.

OK, so can we close the issue? The thing is that coverage is checking your code and not companion code. About the flag it is not that important if it's working for you.

from kotlin-logging.

Yes, thanks for the help!

from kotlin-logging.

I would like to ask to reopen this issue.

Currently the recommended logger setup is not checked by jacoco code coverage. This may affect teams' adoption of this library since it affect their code coverage metric.

Its better to have a discussion on how to solve this issue moving forward

from kotlin-logging.

the above are the 2 screenshots of current jacoco coverage which showed missed instruction

Kotlin-logging 1.6.26
Jacoco 0.8.4
Surefire 3.0.0-M3

from kotlin-logging.

@sluongng thanks for raising the issue again. My suggested solution is to have a JVM arg like -Dkotlin.logging.coverageAggresive and then activate it on the coverage run. What do you think about that? would you like to test that? or do you have other idea?

from kotlin-logging.

@sluongng thanks for raising the issue again. My suggested solution is to have a JVM arg like -Dkotlin.logging.coverageAggresive and then activate it on the coverage run. What do you think about that? would you like to test that? or do you have other idea?

The downside of adding JVM arg is that its a bit hard to check-in this arg into git unless you embeding it into a dockerfile or abstracting your project behind a Makefile layer so I dont find it to be an ideal solution.

I am relative new to kotlin but I think for start, we can just document a pattern to exclude these logging methods in the Jacoco report generator. This pattern then will be check-in into git via maven pom file or gradle build file.

from kotlin-logging.

if you're using gradle or maven for coverage then I think commiting the JVM args to git is possible via its configuration.
Document a pattern is fine from my side, but from what I saw that excludes an entire class.

from kotlin-logging.

learning from Lombok, the generated code could be configured to have a "@generated" annotation attached and Jacoco automatically ignore code with said annotation

not sure if its applicable in this case?

from kotlin-logging.

@sluongng I saw you just opened the issue on jacoco itself. I think this is a good idea (I thought to do it myself) to see how they suggest solving the issue. Thanks!

from kotlin-logging.

as mentioned on jacoco/jacoco#903

adding <exclude>**/*$log$*.class</exclude> into Jacoco configuration seems to solved the issue.

I suggest consider adding this tip into README.md or a wiki page

Thanks, cheers!

from kotlin-logging.

I will add that to the wiki.

from kotlin-logging.