stevesaliman / gradle-cobertura-plugin

Gradle Cobertura Plugin
118 stars 50 forks source link

Make cobertura plugin delete classes which are excluded from instrumentation #160

Closed eyalroth closed 5 years ago

eyalroth commented 5 years ago

In an attempt to make both cobertura and scoverage run together and generate reports while executing the tests only once, it would be very helpful if the cobertura plugin will delete the "instrumented" classes which are configured to be excluded from the instrumentation (the instrumentation process outputs all the classes regardless).

Running both coverage tools together can only be achieved if each tool is responsible for instrumenting an exclusive set of classes; for example, cobertura will instrument java classes while scoverage will instrument scala classes. Thing is, since both of the classes output directories (of both coverage tools) have to be on the classpath of the tests execution, they must not include non-instrumented classes, as those might override the instrumented classes of the other tool.

For more information on this, please refer to the relevant issue on the scoverge gradle plugin repository.

eyalroth commented 5 years ago

I have an idea of two ways for achieving this:

  1. Instead of copying all of the compiled files to the instrumentation directory at first, only copy files which correspond to the include / exclude parameters (the file paths should match the fully qualified names of the classes).
  2. Delete instrumented files which are identical to their counterpart in the original classes directory. This would be preferable since it will support cases where entire classes are excluded with @CoverageIgnore annotations.
stevesaliman commented 5 years ago

I agree that the second option is probably the best one.

I'll try to carve out some time in the next couple of weeks to see what it might take to implement this - and still have the coverage reports do what I'd expect.

eyalroth commented 5 years ago

@stevesaliman Feel free to take a look at my example project which combines cobertura and scoverage. I've implemented the second method there, and it seems that cobertura is generating all the reports correctly, including inner classes.

eyalroth commented 5 years ago

@stevesaliman Note that the latest version of the gradle plugin for scoverage (3.0.0) now deletes non-instrumented files so it's much easier to integrate it with cobertura, but still requires additional configuration in order to make them both work in a given project (see my example project with the branch working with scoverage 3).

The changes were introduced in PR #88 for that repository.

stevesaliman commented 5 years ago

I took a look at how the scoverage plugin does it, as well as your sample project. It doesn't seem too tricky to implement, I could add code to InstrumentTask that runs after cobertura to remove any files that cobertura didn't actually change.

I do have one concern with the specific implementation. It uses the relativePath method from Groovy 2.5. Gradle 4.x uses Groovy2.4, and I don't want to break the plugin for users of Gradle 4. If I get time next weekend, I can see if I can implement this with code that works in Gradle 2.4

eyalroth commented 5 years ago

I do have one concern with the specific implementation. It uses the relativePath method from Groovy 2.5. Gradle 4.x uses Groovy2.4, and I don't want to break the plugin for users of Gradle 4. If I get time next weekend, I can see if I can implement this with code that works in Gradle 2.4

Could perhaps simply replace it with java.nio.file.Path#relativize().

stevesaliman commented 5 years ago

That works, thanks.

I pushed the latest code if you want to try it out. I have another issue I want to resolve before I do a release, which I hope to resolve this weekend.

stevesaliman commented 5 years ago

Version 2.6.1 has now been released with this fix.