Skip to content
This repository was archived by the owner on Mar 27, 2025. It is now read-only.

Commit 5fddcb5

Browse files
nbhoskinbhoski
committed
updated review comments.
1 parent 71c0755 commit 5fddcb5

File tree

5 files changed

+49
-77
lines changed

5 files changed

+49
-77
lines changed

‎src/main/java/com/mathworks/ci/MatlabBuilder.java‎

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,8 @@ public FormValidation getFirstErrorOrWarning(
192192
final MatrixPatternResolver resolver = new MatrixPatternResolver(matlabRoot);
193193
if (!resolver.hasVariablePattern()) {
194194
try {
195-
rel = new MatlabReleaseInfo(matlabRoot);
195+
FilePath matlabRootPath = new FilePath(new File(matlabRoot));
196+
rel = new MatlabReleaseInfo(matlabRootPath);
196197
if (rel.verLessThan(BASE_MATLAB_VERSION_RUNTESTS_SUPPORT)) {
197198
return FormValidation
198199
.error(Message.getValue("Builder.matlab.test.support.error"));
@@ -259,7 +260,8 @@ public FormValidation doCheckTaCoberturaChkBx(@QueryParameter boolean taCobertur
259260
}
260261

261262
Function<String, FormValidation> chkCoberturaSupport = (String matlabRoot) -> {
262-
rel = new MatlabReleaseInfo(matlabRoot);
263+
FilePath matlabRootPath = new FilePath(new File(matlabRoot));
264+
rel = new MatlabReleaseInfo(matlabRootPath);
263265
final MatrixPatternResolver resolver = new MatrixPatternResolver(matlabRoot);
264266
if(!resolver.hasVariablePattern()) {
265267
try {
@@ -395,42 +397,44 @@ public String getStringByName(String memberName) {
395397
public void perform(@Nonnull Run<?, ?> build, @Nonnull FilePath workspace,
396398
@Nonnull Launcher launcher, @Nonnull TaskListener listener)
397399
throws InterruptedException, IOException {
398-
final boolean isLinuxLauncher = launcher.isUnix();
400+
//Set the environment variable specific to the this build
401+
setEnv(build.getEnvironment(listener));
399402
nodeSpecificfileSeparator = getNodeSpecificFileSeperator(launcher);
400403

401404
// Invoke MATLAB command and transfer output to standard
402405
// Output Console
403406

404-
buildResult = execMatlabCommand(build, workspace, launcher, listener, isLinuxLauncher);
407+
buildResult = execMatlabCommand(workspace, launcher, listener);
405408

406409
if (buildResult != 0) {
407410
build.setResult(Result.FAILURE);
408411
}
409412
}
410413

411-
private synchronized int execMatlabCommand(Run<?, ?> build, FilePath workspace, Launcher launcher,
412-
TaskListener listener, boolean isLinuxLauncher)
414+
private synchronized int execMatlabCommand(FilePath workspace, Launcher launcher,
415+
TaskListener listener)
413416
throws IOException, InterruptedException {
414-
setEnv(build.getEnvironment(listener));
415-
final String testRunMode = this.getTestRunTypeList().getDescriptor().getId();
416-
FilePath targetWorkspace = new FilePath(launcher.getChannel(), workspace.getRemote());
417-
418-
// Copy MATLAB scratch file into the workspace only if Automatic option is selected.
419-
if (testRunMode.contains(AUTOMATIC_OPTION)) {
420-
copyMatlabScratchFileInWorkspace(MATLAB_RUNNER_RESOURCE, MATLAB_RUNNER_TARGET_FILE, targetWorkspace);
421-
}
422417
ProcStarter matlabLauncher;
423418
try {
424419
FilePath nodeSpecificMatlabRoot = new FilePath(launcher.getChannel(),getLocalMatlab());
425420
MatlabReleaseInfo rel = new MatlabReleaseInfo(nodeSpecificMatlabRoot);
426421
matlabLauncher = launcher.launch().pwd(workspace).envs(this.env);
427422
if (rel.verLessThan(BASE_MATLAB_VERSION_BATCH_SUPPORT)) {
428423
ListenerLogDecorator outStream = new ListenerLogDecorator(listener);
429-
matlabLauncher = matlabLauncher.cmds(constructDefaultMatlabCommand(isLinuxLauncher)).stderr(outStream);
424+
matlabLauncher = matlabLauncher.cmds(constructDefaultMatlabCommand(launcher.isUnix())).stderr(outStream);
430425
} else {
431426
matlabLauncher = matlabLauncher.cmds(constructMatlabCommandWithBatch()).stdout(listener);
432427
}
433-
} catch (MatlabVersionNotFoundException e) {
428+
429+
//Check the test run mode option selected by user and identify the target workspace to copy the scratch file.
430+
final String testRunMode = this.getTestRunTypeList().getDescriptor().getId();
431+
FilePath targetWorkspace = new FilePath(launcher.getChannel(), workspace.getRemote());
432+
433+
// Copy MATLAB scratch file into the workspace only if Automatic option is selected.
434+
if (testRunMode.contains(AUTOMATIC_OPTION)) {
435+
copyMatlabScratchFileInWorkspace(MATLAB_RUNNER_RESOURCE, MATLAB_RUNNER_TARGET_FILE, targetWorkspace);
436+
}
437+
} catch (Exception e) {
434438
listener.getLogger().println(e.getMessage());
435439
return 1;
436440
}

‎src/main/java/com/mathworks/ci/MatlabReleaseInfo.java‎

Lines changed: 24 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,6 @@ public class MatlabReleaseInfo {
3939
};
4040

4141
private Map<String, String> versionInfoCache = new HashMap<String, String>();
42-
43-
public MatlabReleaseInfo(String matlabRoot) {
44-
this.matlabRoot = new FilePath(new File(matlabRoot));
45-
}
4642

4743
public MatlabReleaseInfo(FilePath matlabRoot) {
4844
this.matlabRoot = matlabRoot;
@@ -84,7 +80,29 @@ private Map<String, String> getVersionInfoFromFile() throws MatlabVersionNotFoun
8480
try {
8581
FilePath versionFile = new FilePath(this.matlabRoot, VERSION_INFO_FILE);
8682
if(versionFile.exists()) {
87-
versionInfoCache.putAll(versionFile.act(new RemoteFileOperation()));
83+
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
84+
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
85+
Document doc = dBuilder.parse(versionFile.read());
86+
87+
doc.getDocumentElement().normalize();
88+
NodeList nList = doc.getElementsByTagName(VERSION_INFO_ROOT_TAG);
89+
90+
for (int temp = 0; temp < nList.getLength(); temp++) {
91+
Node nNode = nList.item(temp);
92+
if (nNode.getNodeType() == Node.ELEMENT_NODE) {
93+
94+
Element eElement = (Element) nNode;
95+
96+
versionInfoCache.put(RELEASE_TAG, eElement.getElementsByTagName(RELEASE_TAG)
97+
.item(0).getTextContent());
98+
versionInfoCache.put(VERSION_TAG, eElement.getElementsByTagName(VERSION_TAG)
99+
.item(0).getTextContent());
100+
versionInfoCache.put(DESCRIPTION_TAG, eElement
101+
.getElementsByTagName(DESCRIPTION_TAG).item(0).getTextContent());
102+
versionInfoCache.put(DATE_TAG,
103+
eElement.getElementsByTagName(DATE_TAG).item(0).getTextContent());
104+
}
105+
}
88106
}
89107
else if(!this.matlabRoot.exists()){
90108
throw new NotDirectoryException("Invalid matlabroot path");
@@ -99,56 +117,4 @@ else if(!this.matlabRoot.exists()){
99117
}
100118
return versionInfoCache;
101119
}
102-
103-
/*
104-
* Static File Callable to perform File operations on specific remote nodes.
105-
*/
106-
107-
private static final class RemoteFileOperation implements FileCallable<Map<String, String>> {
108-
109-
private static final long serialVersionUID = 1L;
110-
111-
private Map<String, String> versionInfoCache = new HashMap<String, String>();
112-
113-
@Override
114-
public void checkRoles(RoleChecker checker) throws SecurityException {
115-
// No checks to perform
116-
}
117-
118-
@SuppressFBWarnings(value = "REC_CATCH_EXCEPTION",
119-
justification = "Irrespective of exception type, intention is to handle it in same way. Also, there is no intention to propagate any runtime exception up in the hierarchy.")
120-
@Override
121-
public Map<String, String> invoke(File versionFile, VirtualChannel channel)
122-
throws IOException, InterruptedException {
123-
124-
try {
125-
126-
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
127-
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
128-
Document doc = dBuilder.parse(versionFile);
129-
doc.getDocumentElement().normalize();
130-
NodeList nList = doc.getElementsByTagName(VERSION_INFO_ROOT_TAG);
131-
132-
for (int temp = 0; temp < nList.getLength(); temp++) {
133-
Node nNode = nList.item(temp);
134-
if (nNode.getNodeType() == Node.ELEMENT_NODE) {
135-
136-
Element eElement = (Element) nNode;
137-
138-
versionInfoCache.put(RELEASE_TAG, eElement.getElementsByTagName(RELEASE_TAG)
139-
.item(0).getTextContent());
140-
versionInfoCache.put(VERSION_TAG, eElement.getElementsByTagName(VERSION_TAG)
141-
.item(0).getTextContent());
142-
versionInfoCache.put(DESCRIPTION_TAG, eElement
143-
.getElementsByTagName(DESCRIPTION_TAG).item(0).getTextContent());
144-
versionInfoCache.put(DATE_TAG,
145-
eElement.getElementsByTagName(DATE_TAG).item(0).getTextContent());
146-
}
147-
}
148-
} catch (Exception e) {
149-
throw new IOException("Error in reading MATLAB VersionInfo file",e);
150-
}
151-
return versionInfoCache;
152-
}
153-
}
154-
}
120+
}

‎src/main/resources/config.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
Builder.display.name = Run MATLAB Tests
55
Builder.matlab.runner.target.file.name = runMatlabTests.m
6-
Builder.matlab.cobertura.support.warning = To generate a Cobertura report, use MATLAB R2017b or a newer version.
6+
Builder.matlab.cobertura.support.warning = To generate a Cobertura report, use MATLAB R2017b or a newer release.
77
Builder.invalid.matlab.root.warning = Unable to find MATLAB from the specified location on this system(but perhaps it exists on some agents)
88
Builder.matlab.root.empty.error = Full path to the MATLAB root folder is required.
99
Builder.matlab.test.support.error = To run tests with the Jenkins plugin, use MATLAB R2013a or a newer version.

‎src/test/java/com/mathworks/ci/MatlabBuilderTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import static org.junit.Assert.assertFalse;
66
import hudson.EnvVars;
7+
import hudson.FilePath;
78
import hudson.model.FreeStyleBuild;
89
import hudson.model.Result;
910
import hudson.slaves.EnvironmentVariablesNodeProperty;
@@ -245,7 +246,8 @@ public void verifyBuildStatusWhenTestFails() throws Exception {
245246

246247
@Test
247248
public void verifyVerlessThan() throws Exception {
248-
MatlabReleaseInfo rel = new MatlabReleaseInfo(getMatlabroot("R2017a"));
249+
FilePath matlabRoot = new FilePath(new File(getMatlabroot("R2017a")));
250+
MatlabReleaseInfo rel = new MatlabReleaseInfo(matlabRoot);
249251

250252
// verLessthan() will check all the versions against 9.2 which is version of R2017a
251253
assertFalse(rel.verLessThan(9.1));

‎src/test/resources/testconfig.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@
33

44
Verify.matlab.invokes.positive = MATLAB is invoking positive tests
55
Verify.build.ignore.test.failure = Build Ignored test failure
6-
Builder.matlab.cobertura.support.warning = To generate a Cobertura report, use MATLAB R2017b or a newer version.
6+
Builder.matlab.cobertura.support.warning = To generate a Cobertura report, use MATLAB R2017b or a newer release.
77
Builder.invalid.matlab.root.warning = Unable to find MATLAB from the specified location on this system(but perhaps it exists on some agents)
88
Builder.matlab.root.empty.error = Full path to the MATLAB root folder is required.

0 commit comments

Comments
 (0)