New flag to enable/disable display of source files with line coverage highlights #81
Conversation
| @@ -451,13 +461,18 @@ public void perform(@Nonnull Run<?, ?> run, @Nonnull FilePath filePath, @Nonnull | |||
| logger.print("\n[JaCoCo plugin] Saving matched class directories for class-pattern: " + classPattern + ": "); | |||
| for (FilePath file : matchedClassDirs) { | |||
| dir.saveClassesFrom(file); | |||
| logger.print(" " + file); | |||
There was a problem hiding this comment.
These log-outputs were done on purpose to help with investigation of problems, any reason why you removed them as part of this PR?
There was a problem hiding this comment.
I have integrated jacoco plugin with a CI build job of a application having thousands of classes. The log statement prints numerous file paths on CI log which seemed unnecessary. For investigation can we add a summary message instead? Please suggest. Thanks.
There was a problem hiding this comment.
these should only be the top-level class dirs, e.g. build/classes, not each separate directory underneath. This sounds like you configured it in a way so that it selected all those separately one-by-one instead of only the top-level dir. Can you try to configure it differently and see if the printout looks sane then while still collecting coverage?
There was a problem hiding this comment.
E.g. you can take a look at the build at https://builds.apache.org/view/POI/job/POI-DSL-1.6/, it has thousands of class-files as well, but we only configure the class-directories "build/classes,build/excelant-classes,build/ooxml-classes,build/scratchpad-classes,build/*/build/classes", this results in the following nice and helpful log-output:
[JaCoCo plugin] Collecting JaCoCo coverage data...
[JaCoCo plugin] build/*.exec,build/*/build/jacoco/*.exec;build/classes,build/excelant-classes,build/ooxml-classes,build/scratchpad-classes,build/*/build/classes;src/java,src/excelant/java,src/ooxml/java,src/scratchpad/src; locations are configured
Configuring it differently so that it needs to iterate over all classes separately might even cause quite an overhead when executing the plugin...
There was a problem hiding this comment.
@centic9 I am in process of verifying your suggestion in my CI job.
There was a problem hiding this comment.
@centic9 Turns out the log message problem was due to a '/' after my class directory name which caused it to iterate over every sub-directory as you mentioned. Proper class directory has reduced the total execution time as well. This was a great help and I appreciate your suggestions. I have added the log messages back as part of this PR.
| logger.print("\n[JaCoCo plugin] Saving matched source directories for source-pattern: " + sourcePattern + ": "); | ||
| for (FilePath file : matchedSrcDirs) { | ||
| dir.saveSourcesFrom(file); | ||
| logger.print(" " + file); |
|
Don't you get the same behaviour by unsetting |
|
Thanks for the PR, this is merged now and will be included in the next release of the plugin. |
|
Thank you @centic9 for accepting the PR. |
Added a new flag named "Disable display of source files for coverage" to allow the end-user to choose whether to copy source files or not. This is a required use-case as some applications have thousands of source files and Jacoco plugin consumes lot of time to copy the files to the master node. Users are not always interested in viewing the line coverage highlights in source files specially when the application have thousands or more files. Providing an option will serve the purpose.
