Add filter for methods that Kotlin compiler generates #689

goodwinnk · 2018-06-05T14:49:56Z

Filtering for Kotlin generated methods based on line numbers absence among method instructions.

Constructors are ignored for tests with debug information stripped pass.

List of class annotations descriptions and source file name are added.

marchof · 2018-06-05T15:00:47Z

@goodwinnk Are generated method already without line numbers? Or what would be the minimum required Kotlin compiler version?

goodwinnk · 2018-06-05T15:28:08Z

Generated methods are without line numbers since Kotlin 1.0.0 till the latest one.

marchof

Hi @goodwinnk , thanks for the great idea how to implement filtering and this well-crafted pull request. I would like to propose two formal changes. We also always add an entry to changes.html.

If you don't mind I can take care of this.

marchof · 2018-06-05T18:11:24Z

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinNoSourceLinesFilter.java

+import java.util.ListIterator;
+import java.util.Set;
+
+public class KotlinNoSourceLinesFilter implements IFilter {


JavaDoc missing.

In addition to a missing Javadoc - maybe also rename into KotlinGeneratedFilter by analogy with existing GroovyGeneratedFilter and LombokGeneratedFilter ?

marchof · 2018-06-05T18:17:06Z

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/IFilter.java

 	 * @param methodNode
 	 *            method to inspect
 	 * @param output
 	 *            callback to report filtering results to
 	 */
-	void filter(String className, String superClassName, MethodNode methodNode,
+	void filter(String className, String superClassName,


Now we're definitely beyond the limit of extra parameters. Probably we will see even more in future. I would propose to introduce a new interface

public interface org.jacoco.core.internal.analysis.filter.IFilterContext{ String getClassName(); String getSuperClassName(); Set<String> getClassAnnotations(); String getSourceFileName(); }

For now this can be directly implemented by ClassAnalyzer.

I thought about introducing something like IFilterContext but decided that it's a bit more structural change than this PR needs.

Godin

First of all - thank you for this nice contribution! 👍 ❤️

I added few more comments. IMO one about iteration and one about cleanup of map are worth to be addressed, others are really minor, picky as @marchof loves 😉 in order to keep our codebase healthy, clean and consistent, so feel free to leave them on his shoulders as he already requested 😜 😆

...co.core.test/src/org/jacoco/core/internal/analysis/filter/TryWithResourcesEcjFilterTest.java

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinNoSourceLinesFilter.java

org.jacoco.core/src/org/jacoco/core/internal/analysis/ClassAnalyzer.java

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/AbstractAnnotatedMethodFilter.java

org.jacoco.core/src/org/jacoco/core/internal/analysis/MethodAnalyzer.java

org.jacoco.core/src/org/jacoco/core/internal/analysis/ClassAnalyzer.java

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/Filters.java

...co.core.test/src/org/jacoco/core/internal/analysis/filter/KotlinNoSourceLinesFilterTest.java

Godin · 2018-06-05T22:15:50Z

org.jacoco.core/src/org/jacoco/core/internal/analysis/filter/KotlinNoSourceLinesFilter.java

+import java.util.ListIterator;
+import java.util.Set;
+
+public class KotlinNoSourceLinesFilter implements IFilter {


In addition to a missing Javadoc - maybe also rename into KotlinGeneratedFilter by analogy with existing GroovyGeneratedFilter and LombokGeneratedFilter ?

* Rename Filter to KotlinGeneratedFilter in analogy to other filters. * Iterate over instructions without allocation * Identify LineNumberNode with getType() * Remove unnecessary clear() * Use project source formatting (final, imports)

marchof · 2018-06-06T11:04:39Z

@Godin I tried to address your issues in first step. I will work on my findings tonight.

marchof · 2018-06-06T11:07:31Z

@goodwinnk Silly question from my side: Have you done some integration testing with a Kotlin project? Are the results as expected?

goodwinnk · 2018-06-06T12:31:44Z

@marchof, @Godin Thank you for the review.
@marchof I didn't notice your changes and prepared some fixes, but they are probably the same (https://github.com/goodwinnk/jacoco/commits/kotlin-generated-filter-fixes).

I would still like to ask you fill changes.html in the way you prefer it.

@goodwinnk Silly question from my side: Have you done some integration testing with a Kotlin project? Are the results as expected?

I'm using the sample project https://github.com/goodwinnk/kotlin-features-coverage and have just updated readme with the results from jacoco with KotlinGeneratedFilter enabled.

All results are as expected.
Inlines are still uncovered because of the #654. Function cannot be considered generated as it written in user code.
There're also problems with coroutines feature (there're uncovered branches in coroutines), but there would be an additional investigation, because coroutines are not finalized feature in Kotlin.

marchof · 2018-06-06T12:48:55Z

@goodwinnk Thanks for the deedback! Sure, I will finalize this PR.

marchof · 2018-06-06T17:20:50Z

@Godin I tried to address all issues . Can you please re-review?

Godin

@marchof perfect! 👍

As usual I also made end-to-end test - example.zip

Before this change using JaCoCo 0.8.1:

After this change:

Results are the same for Kotlin 1.0.1 and 1.2.41.

Noticed that val's in data classes require read test and vars both read and write.
This seems correct to me - don't read val = remove it, don't write var = make it val. And this seems to be consistent with Groovy. However not consistent with Lombok for which we filter out generated setters and getters 😆

hleb-albau · 2018-06-08T10:54:42Z

Hi, thanks !
When new release is planned?

marchof · 2018-06-08T11:25:17Z

See FAQ https://www.jacoco.org/jacoco/trunk/doc/faq.html

When will feature X be released?

JaCoCo is maintained by volunteers in their free time. Since we cannot guarantee free capacity, we do not commit to particular release dates. Typically, you can expect a couple of releases every year.

In the change log, you can see all features that have been been implemented in master branch and > will be available with the next release. And in the meantime you can test latest build of of master branch (Maven SNAPSHOT) and provide feedback to us.

NickButcher1 · 2018-06-08T13:30:49Z

I have pointed my app at 0.8.2-SNAPSHOT. The fix is working nicely, thanks.

hleb-albau · 2018-06-08T13:33:38Z

@marchof I also moved to SNAPSHOT release, no issues, thanks.

PS: Sorry, I should read docs first.

The report currently doesn't look great due to JaCoCo including compiler-generated methods such as equals and hashCode. This will be fixed in the next JaCoCo release (see jacoco/jacoco#689).

Barryrowe · 2018-10-08T20:35:44Z

Is there a configuration option that needs to be set to take advantage of this with 0.8.2?
I have updated a project from 0.8.0 --> 0.8.2 but see no difference in my coverage and data classes still calculate coverage on generated toString()/hashCode() etc. functions.

Godin · 2018-10-08T20:55:46Z

No - all filters so far implemented in JaCoCo are enabled unconditionally and do not require any configuration.

Also by carefully reading this thread - you can find above example.zip which contains complete example for Maven and Gradle demonstrating that toString and hashCode are perfectly filtered out in data classes as shown on corresponding screenshots. This example was used as end-to-end test of unreleased version, however replacement of 0.8.2-SNAPSHOT on 0.8.2 in pom.xml and build.gradle doesn't change result.

Please also pay attention on what is stated in each release announcement starting from 0.8.0 up to 0.8.2 and in changelog:

filtering happens at a time of generation of reports, so that

Integrations developed as part of JaCoCo project by us (Ant Tasks, Maven Plugin and Command Line Interface) provide new filters.

while

Tools that directly read exec files (which is not a final report) and embed JaCoCo for generation of report will provide filtering functionality only after they updated to this version of JaCoCo.

If the above doesn't help, then please use our mailing list to provide complete example that demonstrates your problem.

goodwinnk added 3 commits June 5, 2018 17:24

Initial prototype for filtering out methods without line numbers

3ad8f4a

Constructors are ignored for tests with debug information stripped pass.

Add more information about class to method filters

acb7b90

List of class annotations descriptions and source file name are added.

Enable NoSourceLinesFilter only for Kotlin bytecode

aa53d2c

marchof requested changes Jun 5, 2018

View reviewed changes

Godin added type: enhancement component: core.filters type: experimental 🚧 labels Jun 5, 2018

Godin requested changes Jun 5, 2018

View reviewed changes

Godin added this to IN PROGRESS in Filtering Jun 5, 2018

Godin added this to IN PROGRESS in Current work items Jun 6, 2018

Fix Godin's review comments.

70025c3

* Rename Filter to KotlinGeneratedFilter in analogy to other filters. * Iterate over instructions without allocation * Identify LineNumberNode with getType() * Remove unnecessary clear() * Use project source formatting (final, imports)

marchof removed the type: experimental 🚧 label Jun 6, 2018

marchof added 2 commits June 6, 2018 19:07

Introduce IFilterContext to reduce amount filter parameters.

44a6924

Add change log entry.

a3113eb

marchof approved these changes Jun 6, 2018

View reviewed changes

Godin mentioned this pull request Jun 6, 2018

Recognize and ignore Kotlin-generated code #552

Closed

Godin added this to the 0.8.2 milestone Jun 6, 2018

Godin approved these changes Jun 7, 2018

View reviewed changes

Godin changed the title ~~Kotlin generated filter~~ Add filter for methods that Kotlin compiler generates Jun 7, 2018

Godin merged commit 281538b into jacoco:master Jun 7, 2018

Godin removed this from IN PROGRESS in Current work items Jun 7, 2018

Godin moved this from IN PROGRESS to DONE in Filtering Jun 7, 2018

Godin mentioned this pull request Jun 13, 2018

Ignore Immutables generated code. #656

Closed

Godin mentioned this pull request Jul 10, 2018

Add filter for Kotlin lateinit properties #707

Merged

marshallaf mentioned this pull request Jul 15, 2018

Codacy marshallaf/GithubTrending#9

Merged

Godin mentioned this pull request Aug 20, 2018

Invalid missing line coverage in report #751

Closed

iNoles mentioned this pull request Sep 29, 2018

Change JaCoCo Plugin Version kittinunf/fuel#446

Closed

1 task

jacoco locked as resolved and limited conversation to collaborators Oct 8, 2018

Godin added the language: Kotlin label Oct 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add filter for methods that Kotlin compiler generates #689

Add filter for methods that Kotlin compiler generates #689

goodwinnk commented Jun 5, 2018

marchof commented Jun 5, 2018

goodwinnk commented Jun 5, 2018

marchof left a comment

marchof Jun 5, 2018

Godin Jun 5, 2018

marchof Jun 5, 2018

goodwinnk Jun 6, 2018

Godin left a comment

Godin Jun 5, 2018

marchof commented Jun 6, 2018

marchof commented Jun 6, 2018

goodwinnk commented Jun 6, 2018

marchof commented Jun 6, 2018

marchof commented Jun 6, 2018

Godin left a comment

hleb-albau commented Jun 8, 2018

marchof commented Jun 8, 2018

NickButcher1 commented Jun 8, 2018

hleb-albau commented Jun 8, 2018

Barryrowe commented Oct 8, 2018

Godin commented Oct 8, 2018 •

edited

Loading

Add filter for methods that Kotlin compiler generates #689

Add filter for methods that Kotlin compiler generates #689

Conversation

goodwinnk commented Jun 5, 2018

marchof commented Jun 5, 2018

goodwinnk commented Jun 5, 2018

marchof left a comment

Choose a reason for hiding this comment

marchof Jun 5, 2018

Choose a reason for hiding this comment

Godin Jun 5, 2018

Choose a reason for hiding this comment

marchof Jun 5, 2018

Choose a reason for hiding this comment

goodwinnk Jun 6, 2018

Choose a reason for hiding this comment

Godin left a comment

Choose a reason for hiding this comment

Godin Jun 5, 2018

Choose a reason for hiding this comment

marchof commented Jun 6, 2018

marchof commented Jun 6, 2018

goodwinnk commented Jun 6, 2018

marchof commented Jun 6, 2018

marchof commented Jun 6, 2018

Godin left a comment

Choose a reason for hiding this comment

hleb-albau commented Jun 8, 2018

marchof commented Jun 8, 2018

NickButcher1 commented Jun 8, 2018

hleb-albau commented Jun 8, 2018

Barryrowe commented Oct 8, 2018

Godin commented Oct 8, 2018 • edited Loading

Godin commented Oct 8, 2018 •

edited

Loading