Skip to content

Commit 5f800f2

Browse files
K0zkaK0zka
committed
Script cleanup
- new negative tests - some copy-paste replacement in the code Signed-off-by: Laszlo Hornyak <laszlo.hornyak@gmail.com>
1 parent 6617187 commit 5f800f2

2 files changed

Lines changed: 59 additions & 37 deletions

File tree

‎utils/src/com/cloud/utils/script/Script.java‎

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,6 @@
1919

2020
package com.cloud.utils.script;
2121

22-
import com.cloud.utils.PropertiesUtil;
23-
import com.cloud.utils.concurrency.NamedThreadFactory;
24-
import com.cloud.utils.script.OutputInterpreter.TimedOutLogger;
25-
import org.apache.log4j.Logger;
26-
2722
import java.io.BufferedReader;
2823
import java.io.File;
2924
import java.io.FileInputStream;
@@ -43,6 +38,13 @@
4338
import java.util.concurrent.ScheduledFuture;
4439
import java.util.concurrent.TimeUnit;
4540

41+
import org.apache.commons.io.IOUtils;
42+
import org.apache.log4j.Logger;
43+
44+
import com.cloud.utils.PropertiesUtil;
45+
import com.cloud.utils.concurrency.NamedThreadFactory;
46+
import com.cloud.utils.script.OutputInterpreter.TimedOutLogger;
47+
4648
public class Script implements Callable<String> {
4749
private static final Logger s_logger = Logger.getLogger(Script.class);
4850

@@ -167,6 +169,16 @@ public String toString() {
167169
return buildCommandLine(command);
168170
}
169171

172+
static String stackTraceAsString(Throwable throwable) {
173+
//TODO: a StringWriter is bit to heavy weight
174+
try(StringWriter out = new StringWriter(); PrintWriter writer = new PrintWriter(out);) {
175+
throwable.printStackTrace(writer);
176+
return out.toString();
177+
} catch (IOException e) {
178+
return "";
179+
}
180+
}
181+
170182
public String execute(OutputInterpreter interpreter) {
171183
String[] command = _command.toArray(new String[_command.size()]);
172184

@@ -259,28 +271,15 @@ public String execute(OutputInterpreter interpreter) {
259271
return error;
260272
} catch (SecurityException ex) {
261273
_logger.warn("Security Exception....not running as root?", ex);
262-
StringWriter writer = new StringWriter();
263-
ex.printStackTrace(new PrintWriter(writer));
264-
return writer.toString();
274+
return stackTraceAsString(ex);
265275
} catch (Exception ex) {
266276
_logger.warn("Exception: " + buildCommandLine(command), ex);
267-
StringWriter writer = new StringWriter();
268-
ex.printStackTrace(new PrintWriter(writer));
269-
return writer.toString();
277+
return stackTraceAsString(ex);
270278
} finally {
271279
if (_process != null) {
272-
try {
273-
_process.getErrorStream().close();
274-
} catch (IOException ex) {
275-
}
276-
try {
277-
_process.getOutputStream().close();
278-
} catch (IOException ex) {
279-
}
280-
try {
281-
_process.getInputStream().close();
282-
} catch (IOException ex) {
283-
}
280+
IOUtils.closeQuietly(_process.getErrorStream());
281+
IOUtils.closeQuietly(_process.getOutputStream());
282+
IOUtils.closeQuietly(_process.getInputStream());
284283
_process.destroy();
285284
}
286285
}
@@ -318,23 +317,15 @@ public void run() {
318317
try {
319318
result = interpreter.interpret(reader);
320319
} catch (IOException ex) {
321-
StringWriter writer = new StringWriter();
322-
ex.printStackTrace(new PrintWriter(writer));
323-
result = writer.toString();
320+
result = stackTraceAsString(ex);
324321
} catch (Exception ex) {
325-
StringWriter writer = new StringWriter();
326-
ex.printStackTrace(new PrintWriter(writer));
327-
result = writer.toString();
322+
result = stackTraceAsString(ex);
328323
} finally {
329324
synchronized (this) {
330325
done = true;
331326
notifyAll();
332327
}
333-
try {
334-
reader.close();
335-
} catch (IOException ex) {
336-
}
337-
;
328+
IOUtils.closeQuietly(reader);
338329
}
339330
}
340331

‎utils/test/com/cloud/utils/ScriptTest.java‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
package com.cloud.utils;
2121

22+
import java.io.BufferedReader;
23+
import java.io.IOException;
24+
2225
import org.apache.commons.lang.SystemUtils;
2326
import org.apache.log4j.Logger;
2427
import org.junit.Assert;
@@ -68,7 +71,8 @@ public void testSet() {
6871
script.add("foo");
6972
script.add("bar", "baz");
7073
script.set("blah", "blah");
71-
Assert.assertEquals("/bin/echo foo bar baz blah blah ", script.toString());
74+
Assert.assertEquals("/bin/echo foo bar baz blah blah ",
75+
script.toString());
7276
}
7377

7478
@Test
@@ -86,13 +90,40 @@ public void testExecute() {
8690
@Test
8791
public void testRunSimpleBashScript() {
8892
Assume.assumeTrue(SystemUtils.IS_OS_LINUX);
89-
Assert.assertEquals("hello world!", Script.runSimpleBashScript("echo 'hello world!'"));
93+
Assert.assertEquals("hello world!",
94+
Script.runSimpleBashScript("echo 'hello world!'"));
95+
}
96+
97+
@Test
98+
public void executeWithOutputInterpreter() {
99+
Assume.assumeTrue(SystemUtils.IS_OS_LINUX);
100+
Script script = new Script("/bin/bash");
101+
script.add("-c");
102+
script.add("echo 'hello world!'");
103+
String value = script.execute(new OutputInterpreter() {
104+
105+
@Override
106+
public String interpret(BufferedReader reader) throws IOException {
107+
throw new IllegalArgumentException();
108+
}
109+
});
110+
// it is a stack trace in this case as string
111+
Assert.assertNotNull(value);
112+
}
113+
114+
@Test
115+
public void runSimpleBashScriptNotExisting() {
116+
Assume.assumeTrue(SystemUtils.IS_OS_LINUX);
117+
String output = Script.runSimpleBashScript("/not/existing/scripts/"
118+
+ System.currentTimeMillis());
119+
Assert.assertNull(output);
90120
}
91121

92122
@Test
93123
public void testRunSimpleBashScriptWithTimeout() {
94124
Assume.assumeTrue(SystemUtils.IS_OS_LINUX);
95-
Assert.assertEquals("hello world!", Script.runSimpleBashScript("echo 'hello world!'", 1000));
125+
Assert.assertEquals("hello world!",
126+
Script.runSimpleBashScript("echo 'hello world!'", 1000));
96127
}
97128

98129
@Test

0 commit comments

Comments
 (0)