Address review comments

Author: abhargav-mw

src/main/java/com/mathworks/ci/MatlabBuild.java

@@ -8,6 +8,7 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.List;
 import java.util.UUID;
 import hudson.EnvVars;
 import hudson.FilePath;
@@ -122,4 +123,9 @@ default String getValidMatlabFileName(String actualName) {
         return MatlabBuilderConstants.MATLAB_TEST_RUNNER_FILE_PREFIX
                 + actualName.replaceAll("-", "_");
     }
+
+    default String getCellArrayFrmList(List<String> listOfStr){
+        listOfStr.replaceAll(val -> "'" + val.replaceAll("'", "''") + "'");
+        return "{" + String.join(",", listOfStr) + "}";
+    }
 }

src/main/java/com/mathworks/ci/RunMatlabTestsBuilder.java

@@ -9,10 +9,21 @@
  */
 
 import java.io.IOException;
-import java.util.*;
+import java.util.Map;
+import java.util.HashMap;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Optional;
+import java.util.Arrays;
 import javax.annotation.Nonnull;
-import hudson.*;
-import hudson.model.*;
+import hudson.EnvVars;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.Launcher;
+import hudson.model.AbstractProject;
+import hudson.model.Result;
+import hudson.model.Run;
+import hudson.model.TaskListener;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.StaplerRequest;
@@ -26,7 +37,8 @@ public class RunMatlabTestsBuilder extends Builder implements SimpleBuildStep, M
 
     private int buildResult;
     private EnvVars env;
-    
+    private static final String SOURCE_FOLDER = "SourceFolder";
+
     // Make all old values transient which protects them writing back on disk.
     private transient boolean tapChkBx;
     private transient boolean junitChkBx;
@@ -289,11 +301,17 @@ private String getInputArguments() {
 
         // Add source folder options to argument
         SourceFolder sf = getSourceFolder();
-        if(sf != null && sf.getSourceFolderPaths().size() != 0){
-            sf.addFilePathArgTo(args);
+        if(sf != null && !sf.getSourceFolderPaths().isEmpty()){
+            sf.addFilePathArgTo(SOURCE_FOLDER, args);
         }
 
-        args.forEach((key, val) -> inputArgsList.add("'" + key + "'" + "," + "'" + val + "'"));
+        args.forEach((key, val) -> {
+            if(key.equals(SOURCE_FOLDER)){
+                inputArgsList.add("'" + key + "'" + "," + val);
+            }else{
+                inputArgsList.add("'" + key + "'" + "," + "'" + val.replaceAll("'", "''") + "'");
+            }
+        });
 
         return String.join(",", inputArgsList);
     }

src/main/java/com/mathworks/ci/RunMatlabTestsStep.java

@@ -24,16 +24,15 @@
 import hudson.model.TaskListener;
 import hudson.Util;
 
-public class RunMatlabTestsStep extends Step {
+public class RunMatlabTestsStep extends Step implements MatlabBuild{
 
     private String testResultsPDF;
     private String testResultsTAP;
     private String testResultsJUnit;
     private String codeCoverageCobertura;
     private String testResultsSimulinkTest;
     private String modelCoverageCobertura;
-    private List<String> sourceFolder;
-  
+    private List<String> sourceFolder = new ArrayList<>();
 
     @DataBoundConstructor
     public RunMatlabTestsStep() {
@@ -104,7 +103,6 @@ public void setSourceFolder(List<String> sourceFolder) {
         this.sourceFolder = Util.fixNull(sourceFolder);
     }
 
-
     @Override
     public StepExecution start(StepContext context) throws Exception {
         return new MatlabRunTestsStepExecution(context, getInputArgs());
@@ -137,7 +135,9 @@ private String getInputArgs() {
         inputArgs.add("'Test'");
 
         args.forEach((key, val) -> {
-            if (val != null) {
+            if(key.equals("SourceFolder") && val != null){
+                inputArgs.add("'" + key + "'" + "," + val);
+            }else if(val != null){
                 inputArgs.add("'" + key + "'" + "," + "'" + val.replaceAll("'", "''") + "'");
             }
         });
@@ -153,11 +153,9 @@ private Map<String, String> getGenscriptArgs() {
         args.put("SimulinkTestResults", getTestResultsSimulinkTest());
         args.put("CoberturaCodeCoverage", getCodeCoverageCobertura());
         args.put("CoberturaModelCoverage", getModelCoverageCobertura());
-        if(getSourceFolder() != null && getSourceFolder().size() != 0){
-            String sourceStr = String.join(";", getSourceFolder());
-            args.put("SourceFolder", sourceStr);
+        if(!getSourceFolder().isEmpty()){
+            args.put("SourceFolder", getCellArrayFrmList(getSourceFolder()));
         }
-
         return args;
     }
 }

src/main/java/com/mathworks/ci/SourceFolder.java

@@ -1,7 +1,7 @@
 package com.mathworks.ci;
 
 /**
- * Copyright 2019-2020 The MathWorks, Inc.
+ * Copyright 2020 The MathWorks, Inc.
  *
  * Describable class for Source Folder Option in RunMATLABTest Build step.
  *
@@ -12,36 +12,28 @@
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import org.kohsuke.stapler.DataBoundConstructor;
-import org.kohsuke.stapler.DataBoundSetter;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
 
-public class SourceFolder extends AbstractDescribableImpl<SourceFolder> {
+public class SourceFolder extends AbstractDescribableImpl<SourceFolder> implements MatlabBuild {
 
-    private List<SourceFolderPaths> sourceFolderPaths = new ArrayList<>();
-    private static final String SOURCE_FOLDER = "SourceFolder";
+    private List<SourceFolderPaths> sourceFolderPaths;
 
     @DataBoundConstructor
-    public SourceFolder() {
-
-    }
-
-    @DataBoundSetter
-    public void setSourceFolderPaths(List<SourceFolderPaths> sourceFolderPaths) {
+    public SourceFolder(List<SourceFolderPaths> sourceFolderPaths) {
         this.sourceFolderPaths = Util.fixNull(sourceFolderPaths);
     }
 
     public List<SourceFolderPaths> getSourceFolderPaths() {
         return this.sourceFolderPaths;
     }
 
-    public  void addFilePathArgTo(Map<String, String> inputArgs) {
-        // Concatenate all source folders to a single ";" separated string
-        inputArgs.put(SOURCE_FOLDER, this.sourceFolderPaths.stream()
+    public  void addFilePathArgTo(String sourceKeyVal, Map<String, String> inputArgs) {
+        // Concatenate all source folders to MATLAB cell array string.
+        inputArgs.put(sourceKeyVal, getCellArrayFrmList(this.sourceFolderPaths.stream()
                 .map(SourceFolderPaths::getSrcFolderPath)
-                .collect(Collectors.joining(";")));
+                .collect(Collectors.toList())));
     }
 
     @Extension public static class DescriptorImpl extends Descriptor<SourceFolder> {

src/main/java/com/mathworks/ci/SourceFolderPaths.java

@@ -1,7 +1,7 @@
 package com.mathworks.ci;
 
 /**
- * Copyright 2019-2020 The MathWorks, Inc.
+ * Copyright 2020 The MathWorks, Inc.
  *
  * Describable class for Repeatable Source Folder text boxes in Source Folder option
  * in RunMATLABTest Build step.

src/main/resources/com/mathworks/ci/RunMatlabTestsBuilder/help-sourceFolder.html

@@ -1,4 +1,4 @@
 <div>
-    <p>Optionally specify the location of the folder containing source code, relative to the project root folder. The specified folder and its subfolders are added to the top of the MATLAB search path. To generate a code coverage report, MATLAB uses only the source code in the specified folder and its subfolders. </p>
-    <p>Specify a source folder path in the input box. Click on 'Add folder' to add more values.</p>
+    <p>Specify the location of folders containing source code, relative to the project root folder. The specified folders and their subfolders are added to the top of the MATLAB search path. To generate a coverage report, MATLAB uses only the source code in the specified folders and their subfolders.</p>
+    <p>Paste the source folder path in the File path box. To add more folders, click Add folder.</p>
 </div>

src/main/resources/com/mathworks/ci/SourceFolder/config.jelly

@@ -1,13 +1,10 @@
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
     <f:entry title="Folder path:">
-        <f:repeatable minimum="1" field="sourceFolderPaths" noAddButton="true">
+        <f:repeatable minimum="1" name="sourceFolderPaths" field="sourceFolderPaths" add="Add folder">
             <table width="100%">
                 <f:textbox field="srcFolderPath"/>
-                <div align ="right">
-                    <input type="button" value="Add folder" class="repeatable-add show-if-last" />
-                    <input type="button" value="Delete" class="repeatable-delete show-if-not-only" />
-                </div>
+                    <input type="button" value="Delete" class="repeatable-delete show-if-not-only" style="margin-left: 0px; margin-top: 0px; padding-top: 0px"/>
             </table>
         </f:repeatable>
     </f:entry>

src/test/java/com/mathworks/ci/RunMatlabTestBuilderPersistenceTest.java

@@ -1,6 +1,6 @@
 package com.mathworks.ci;
 /**
- * Copyright 2019-2020 The MathWorks, Inc.
+ * Copyright 2020 The MathWorks, Inc.
  *
  * Test class for RunMatlabTestsBuilder Persistence
  *
@@ -101,8 +101,7 @@ public void verifySourceFolderSpecPersistence() {
 
             FreeStyleProject project = r.createFreeStyleProject();
             RunMatlabTestsBuilder testBuilder = new RunMatlabTestsBuilder();
-            SourceFolder sf = new SourceFolder();
-            sf.setSourceFolderPaths(paths);
+            SourceFolder sf = new SourceFolder(paths);
             testBuilder.setSourceFolder(sf);
             project.getBuildersList().add(testBuilder);
             project.save();