Added support for Jenkins environment variables.

[nbhoski]

Added Jenkins Environment variable support for both MATLAB root field and Custom command field.
Added unit tests for the same.

[jwilkick-mw]

Would it be possible to use Java's Path object here to build up this value? String concatenation can sometimes be a code smell that perhaps could be handled another way.

[nbhoski]

Yes we will have to use FilePath from hudson. this requires the overall changes across code which uses this string . I am planning to do that during remote execution support.

[mw-akumar]

I have 2 basic concerns with this change

1. We are passing env in too many places, and there doesn't seem to be pattern. It is basically on an as-needed basis

2. The calls to env.expand is sprinkled all through the code. This makes things challenging from a maintenance perspective. As every new line of code is added, you will have to think about whether env.expand needs to be called on it, or not. I think this needs to be encapsulated better.

[mw-akumar]

Add a comment explaining this logic.

[SangaviJayagopi]

Nikhil, Running mvn clean on the latest code, I got this on the log, want to make sure if there is something I am missing. It says there is a recursive loop at in MatlabBuilder.java(see highlighted in logs): private String getLocalMatlab() { return this.env == null ? getLocalMatlab(): this.env.expand(getMatlabRoot()); } Copy pasting the tail end of the log: [INFO] [INFO] ------------------------------------------------------- [INFO] T E S T S [INFO] ------------------------------------------------------- [INFO] Running InjectedTest [INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 6.478 s - in InjectedTest [INFO] Running com.mathworks.ci.MatlabBuilderTest [INFO] Tests run: 23, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 73.554 s - in com.mathworks.ci.MatlabBuilderTest [INFO] [INFO] Results: [INFO] [INFO] Tests run: 31, Failures: 0, Errors: 0, Skipped: 0 [INFO] [INFO] [INFO] --- maven-license-plugin:1.7:process (default) @ matlab --- [INFO] Generated /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab/WEB-INF/licenses.xml [INFO] [INFO] --- maven-hpi-plugin:2.2:hpi (default-hpi) @ matlab --- [INFO] Generating /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab/META-INF/MANIFEST.MF [INFO] Checking for attached .jar artifact ... [INFO] Generating jar /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab.jar [INFO] Building jar: /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab.jar [INFO] Exploding webapp... [INFO] Copy webapp webResources to /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab [INFO] Assembling webapp matlab in /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab [INFO] Generating hpi /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab.hpi [INFO] Building jar: /Users/sjayagop/Desktop/jenkins-matlab-plugin/target/matlab.hpi [INFO] [INFO] --- maven-jar-plugin:3.0.2:test-jar (maybe-test-jar) @ matlab --- [INFO] Skipping packaging of the test-jar [INFO] [INFO] >>> findbugs-maven-plugin:3.0.5:check (findbugs) > :findbugs @ matlab >>> [INFO] [INFO] --- findbugs-maven-plugin:3.0.5:findbugs (findbugs) @ matlab --- [INFO] Fork Value is true [java] Warnings generated: 1 [INFO] Done FindBugs Analysis.... [INFO] [INFO] <<< findbugs-maven-plugin:3.0.5:check (findbugs) < :findbugs @ matlab <<< [INFO] [INFO] [INFO] --- findbugs-maven-plugin:3.0.5:check (findbugs) @ matlab --- [INFO] BugInstance size is 1 [INFO] Error size is 0 [INFO] Total bugs: 1 [INFO] There is an apparent infinite recursive loop in com.mathworks.ci.MatlabBuilder.getLocalMatlab() [com.mathworks.ci.MatlabBuilder] At MatlabBuilder.java:[line 93] IL_INFINITE_RECURSIVE_LOOP [INFO] To see bug detail using the Findbugs GUI, use the following command "mvn findbugs:gui" [INFO] ------------------------------------------------------------------------ [INFO] BUILD FAILURE [INFO] ------------------------------------------------------------------------ [INFO] Total time: 01:42 min [INFO] Finished at: 2019-08-05T16:13:55-04:00 [INFO] ------------------------------------------------------------------------ [ERROR] Failed to execute goal org.codehaus.mojo:findbugs-maven-plugin:3.0.5:check (findbugs) on project matlab: failed with 1 bugs and 0 errors -> [Help 1] [ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. [ERROR] Re-run Maven using the -X switch to enable full debug logging. [ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles: [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException Thanks, Sangavi From: Nikhil Bhoski <notifications@github.com> Reply-To: mathworks/jenkins-matlab-plugin <reply@reply.github.com> Date: Monday, August 5, 2019 at 2:27 AM To: mathworks/jenkins-matlab-plugin <jenkins-matlab-plugin@noreply.github.com> Cc: Sangavi Jayagopi <sjayagop@mathworks.com>, Review requested <review_requested@noreply.github.com> Subject: Re: [mathworks/jenkins-matlab-plugin] Added support for Jenkins environment variables. (#26) Merged #26<#26> into master. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub<#26?email_source=notifications&email_token=AGSA2N6WTK3BSAO3NSX4OH3QC7B5XA5CNFSM4IFE3N32YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOS3Y5MQQ#event-2532431426>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AGSA2NYKJYR45GFZ2CFOOUTQC7B5XANCNFSM4IFE3N3Q>.