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

Add support for environment variables in source and class text fields in the plugin's configuration #77

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

NonGrate
Copy link

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

Copy link
Member

@centic9 centic9 left a 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);
}
Copy link
Member

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?

Copy link
Author

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);
}
Copy link
Member

Choose a reason for hiding this comment

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

dito

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@NonGrate
Copy link
Author

Haven't wrote test yet. Planning to do that.

@don-vip
Copy link
Contributor

don-vip commented Nov 7, 2017

@NonGrate are you still planning to finish the PR? I'd be glad to help.

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