Copilot commented on code in PR #2431:
URL: https://github.com/apache/groovy/pull/2431#discussion_r3035521978
##########
src/main/java/org/codehaus/groovy/runtime/ProcessGroovyMethods.java:
##########
@@ -282,6 +285,99 @@ public static void waitForProcessOutput(Process self,
OutputStream output, Outpu
}
}
+ /**
+ * Executes the process and waits for it to complete, capturing
+ * the standard output, standard error, and exit code into a
+ * {@link ProcessResult}.
+ *
+ * <pre>
+ * def result = "ls -la".execute().waitForResult()
+ * if (result.ok) println result.out
+ * else System.err.println result.err
+ * </pre>
+ *
+ * @param self a Process
+ * @return a ProcessResult containing stdout, stderr, and exit code
+ * @throws InterruptedException if the current thread is interrupted.
+ * @since 6.0.0
+ */
+ public static ProcessResult waitForResult(Process self) throws
InterruptedException {
+ StringBuilder sout = new StringBuilder();
+ StringBuilder serr = new StringBuilder();
+ Thread tout = consumeProcessOutputStream(self, sout);
+ Thread terr = consumeProcessErrorStream(self, serr);
+ boolean interrupted = false;
+ try {
+ try { tout.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ try { terr.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ try { self.waitFor(); } catch (InterruptedException ignore) {
interrupted = true; }
+ closeStreams(self);
+ } finally {
+ if (interrupted) Thread.currentThread().interrupt();
+ }
+ return new ProcessResult(sout.toString(), serr.toString(),
self.exitValue());
+ }
+
+ /**
+ * Executes the process and waits for it to complete within the given
timeout,
+ * capturing the standard output, standard error, and exit code into a
+ * {@link ProcessResult}. If the process does not complete within the
timeout,
+ * it is forcibly destroyed.
+ *
+ * <pre>
+ * def result = "cmd".execute().waitForResult(30, TimeUnit.SECONDS)
+ * </pre>
+ *
+ * @param self a Process
+ * @param timeout the maximum time to wait
+ * @param unit the time unit of the timeout argument
+ * @return a ProcessResult containing stdout, stderr, and exit code
+ * @throws InterruptedException if the current thread is interrupted.
+ * @since 6.0.0
+ */
+ public static ProcessResult waitForResult(Process self, long timeout,
TimeUnit unit) throws InterruptedException {
+ StringBuilder sout = new StringBuilder();
+ StringBuilder serr = new StringBuilder();
+ Thread tout = consumeProcessOutputStream(self, sout);
+ Thread terr = consumeProcessErrorStream(self, serr);
+ boolean interrupted = false;
+ try {
+ try {
+ boolean finished = self.waitFor(timeout, unit);
+ if (!finished) {
+ self.destroyForcibly();
+ self.waitFor();
+ }
+ } catch (InterruptedException ignore) { interrupted = true; }
+ try { tout.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ try { terr.join(); } catch (InterruptedException ignore) {
interrupted = true; }
+ closeStreams(self);
+ } finally {
Review Comment:
`waitForResult(Process,long,TimeUnit)` also swallows `InterruptedException`
even though it is declared to throw it. In the timeout branch this can be
particularly problematic because an interrupt during `waitFor(timeout, unit)`
skips the destroy path and then proceeds to `join()`, potentially blocking
indefinitely while only restoring the interrupt flag. Consider rethrowing on
interrupt (and/or destroying the process) so callers can reliably handle
interruption.
##########
src/test/groovy/groovy/execute/ExecuteTest.groovy:
##########
@@ -167,4 +178,225 @@ final class ExecuteTest {
assert out.toString().contains('hello')
assert process.exitValue() == 0
}
+
+ // --- toProcessBuilder ---
+
+ @Test
+ void testToProcessBuilderFromString() {
+ def pb = cmd.toProcessBuilder()
+ assert pb instanceof ProcessBuilder
+ def process = pb.start()
+ process.waitForProcessOutput()
+ assert process.exitValue() == 0
+ }
+
+ @Test
+ void testToProcessBuilderFromArray() {
+ def pb = cmd.split(' ').toProcessBuilder()
+ assert pb instanceof ProcessBuilder
+ def process = pb.start()
+ process.waitForProcessOutput()
+ assert process.exitValue() == 0
+ }
+
+ @Test
+ void testToProcessBuilderFromList() {
+ def pb = Arrays.asList(cmd.split(' ')).toProcessBuilder()
+ assert pb instanceof ProcessBuilder
+ def process = pb.start()
+ process.waitForProcessOutput()
+ assert process.exitValue() == 0
+ }
+
+ // --- waitForResult ---
+
+ @Test
+ void testWaitForResultFromString() {
+ ProcessResult result = "$echoCmd hello".execute().waitForResult()
+ assert result.ok
+ assert result.exitCode == 0
+ assert result.out.trim().contains('hello')
+ assert result.err != null
+ }
+
+ @Test
+ void testWaitForResultFromArray() {
+ ProcessResult result = (echoCmd.split(' ') +
'hello').execute().waitForResult()
+ assert result.ok
+ assert result.out.trim().contains('hello')
+ }
+
+ @Test
+ void testWaitForResultFromList() {
+ ProcessResult result = ([*echoCmd.split(' '), 'hello'] as
List).execute().waitForResult()
+ assert result.ok
+ assert result.out.trim().contains('hello')
+ }
+
+ @Test
+ void testWaitForResultWithTimeout() {
+ ProcessResult result = "$echoCmd fast".execute().waitForResult(30,
TimeUnit.SECONDS)
+ assert result.ok
+ assert result.out.trim().contains('fast')
+ }
+
+ @Test
+ void testWaitForResultToString() {
+ ProcessResult result = "$echoCmd test".execute().waitForResult()
+ def str = result.toString()
+ assert str.startsWith('ProcessResult(exitCode=')
+ assert str.contains('out=')
+ assert str.contains('err=')
+ }
+
+ // --- execute with Map options ---
+
+ @Test
+ void testExecuteWithDirOption() {
+ def tmpDir = File.createTempDir()
+ try {
+ ProcessResult result = cmd.execute(dir: tmpDir).waitForResult()
+ assert result.ok
+ } finally {
+ tmpDir.deleteDir()
+ }
+ }
+
+ @Test
+ void testExecuteWithDirOptionAsString() {
+ def tmpDir = File.createTempDir()
+ try {
+ ProcessResult result = cmd.execute(dir:
tmpDir.absolutePath).waitForResult()
+ assert result.ok
+ } finally {
+ tmpDir.deleteDir()
+ }
+ }
+
+ @Test
+ void testExecuteWithEnvOption() {
+ List<String> java = [
+ System.getProperty('java.home') + '/bin/java',
+ '-classpath',
+ System.getProperty('java.class.path'),
+ 'groovy.ui.GroovyMain',
+ '-e',
+ "println(System.getenv('MY_TEST_VAR'))"
+ ]
+ ProcessResult result = java.execute(env: [MY_TEST_VAR:
'groovy_test']).waitForResult()
+ assert result.ok
+ assert result.out.trim().contains('groovy_test')
+ }
+
+ @Test
+ void testExecuteWithRedirectErrorStream() {
+ List<String> java = [
+ System.getProperty('java.home') + '/bin/java',
+ '-classpath',
+ System.getProperty('java.class.path'),
+ 'groovy.ui.GroovyMain',
+ '-e',
+ "System.err.println('errMsg'); println('outMsg')"
+ ]
+ ProcessResult result = java.execute(redirectErrorStream:
true).waitForResult()
+ assert result.out.contains('errMsg')
+ assert result.out.contains('outMsg')
+ }
+
+ @Test
+ void testExecuteMapFromArray() {
+ def tmpDir = File.createTempDir()
+ try {
+ ProcessResult result = cmd.split(' ').execute(dir:
tmpDir).waitForResult()
+ assert result.ok
+ } finally {
+ tmpDir.deleteDir()
+ }
+ }
+
+ @Test
+ void testExecuteWithOutputFile() {
+ def tmpDir = File.createTempDir()
+ try {
+ def outFile = new File(tmpDir, 'output.txt')
+ def process = "$echoCmd captured".execute(outputFile: outFile)
+ process.waitFor()
+ assert outFile.text.trim().contains('captured')
+ } finally {
+ tmpDir.deleteDir()
+ }
+ }
+
+ @Test
+ void testExecuteWithInputFile() {
+ def tmpDir = File.createTempDir()
+ try {
+ def inFile = new File(tmpDir, 'input.txt')
+ inFile.text = 'hello from file'
+ def catCmd = System.properties.'os.name'.startsWith('Windows ')
+ ? ['cmd', '/c', 'type', 'CON']
Review Comment:
On Windows, `['cmd','/c','type','CON']` reads from the console device
(`CON`) rather than stdin, so redirecting `inputFile:` may not feed `inFile`
into the process and can hang or produce no output. Use a command that actually
consumes stdin (e.g. `cmd /c more`) so the test reliably validates `inputFile`
redirection on Windows.
```suggestion
? ['cmd', '/c', 'more']
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
