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

Make Clover tasks compatible with the build cache #149

Merged
merged 15 commits into from
Aug 3, 2020

Conversation

ghale
Copy link
Contributor

@ghale ghale commented Jul 31, 2020

These changes make the Clover tasks compatible with the Gradle build cache. There are a few breaking changes, so this would probably require a major version bump. The following are the most impactful changes:

  • These changes drop support for Gradle 2.x and 3.x. Only Gradle 4.x, 5.x and 6.x are now supported.
  • Code instrumentation now occurs in a separate task and writes instrumented class files to a separate directory. The test task now removes the original class directory from its classpath and adds the instrumented class directory to its classpath. There are various changes to CloverSourceSet to support this. Specifically, CloverSourceSet.classesDir no longer represents both the original class dir and the instrumented class dir. classesDir now represents the original (non-instrumented) class directory and CloverSourceSet.instrumentedClassesDir represents the instrumented class directory.
  • In previous versions of this plugin, if cloverInstrumentedJar is set, any jar file that included the main sourceset output would automatically add the instrumented classes (because these classes overwrote the original class files). Now, we automatically configure the jar task from the java plugin, but any other custom jar tasks would have to explicitly add the instrumented classes dir if they want them.
  • A big part of this work is to eliminate the overlapping outputs between tasks as this is not compatible with the build cache. For the most part, these overlapping outputs are somewhat transparent to the user, but one place where we can't avoid it is with the reports directory in multiproject builds. In previous versions, merging the aggregate reports would overwrite any reports written in the report directory of the root project. With these changes, we write the merged aggregate reports to a different location than the reports from the root project. The reports from the root project itself still go into build/reports/clover while the merged aggregate reports now go into build/reports-all/clover in the root project.
  • The same is true of the merged aggregate Clover database. Instead of overwriting the Clover database in the root project, it now writes the merged database to build/.clover/clover.db-all.

@Alex-Vol-SV
Copy link
Collaborator

Wow @ghale you did not disappoint! This is a fantastic change. In fact it probably takes care of the issues I was already aware of including the problem that a test task might do the swapping of classes in a multiproject build while another java sibling project is attempting to compile against the original classes.

It will take me some time to digest your changes but the overall approach is very similar to the conclusions I had made as I was pondering the solution to the problems.

// Add instrumentation task
def instrumentCodeTask = project.tasks.create(getInstrumentationTaskName(test), CloverInstrumentationTask, cloverPluginConvention, test, resolver)
instrumentCodeTask.dependsOn(test.testClassesDirs)
test.dependsOn(instrumentCodeTask)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very elegant solution to the problem @ghale

@Alex-Vol-SV
Copy link
Collaborator

@ghale if you have no further updates I will squash the commits and merge to master in preparation for release. I have some additional changes I was working on that I plan to add before I publish the 3.0.0 release with this. I will update the README/RELEASE_NOTES with the new features/capabilities after I confirm what other open issues are fixed with this.

@ghale
Copy link
Contributor Author

ghale commented Aug 3, 2020

Yes, I'm done with updates unless any other issues surface.

@Alex-Vol-SV Alex-Vol-SV merged commit 290c5d8 into bmuschko:master Aug 3, 2020
@Alex-Vol-SV
Copy link
Collaborator

Thanks @ghale for this great change. I appreciate the work that went into it.
I plan to have this in the 3.0.0 release by the weekend if not sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants