-
Notifications
You must be signed in to change notification settings - Fork 179
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
Add support for environment variables in source and class text fields in the plugin's configuration #77
base: master
Are you sure you want to change the base?
Add support for environment variables in source and class text fields in the plugin's configuration #77
Conversation
… in the plugin's configuration
…irPaths() method and its calls
…ng class and sources paths
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please seem my comment/question.
Also a basic unit-test would be good to verify and cover the new feature. You know, high coverage is good, that is what this plugin is all about :)
FilePath[] matchedClassDirs = {}; | ||
if (run instanceof AbstractBuild) { | ||
matchedClassDirs = resolveDirPaths((AbstractBuild) run, filePath, taskListener, classPattern); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the Jenkins internas here, but shouldn't we keep the previous behaviour if run is not an AbstractBuild?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
FilePath[] matchedSrcDirs = {}; | ||
if (run instanceof AbstractBuild) { | ||
matchedSrcDirs = resolveDirPaths((AbstractBuild) run, filePath, taskListener, sourcePattern); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dito
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Haven't wrote test yet. Planning to do that. |
@NonGrate are you still planning to finish the PR? I'd be glad to help. |
I've met the problem, that when environment variables are used in the JaCoCo configuration in Jenkins.
Exclusions variables is parsed and the value is sent to the JaCoCo report generation, but source and classes paths are passed to the report generation as strings.
This pull request fixes the bug and allows to use environment variables in the class paths and source paths test fields.
Here's the opened issue:
https://issues.jenkins-ci.org/browse/JENKINS-36168