Add CommandRunner and JvmCommandRunner in SystemCommandTasklet

This improves the testability of `SystemCommandTasklet` when the
target command is not available during test execution.

Resolves #3955

Author: scordio

spring-batch-core/src/main/java/org/springframework/batch/core/launch/support/CommandLineJobRunner.java

@@ -181,7 +181,6 @@ public class CommandLineJobRunner {
 
 	private JobLocator jobLocator;
 
-	// Package private for unit test
 	private static SystemExiter systemExiter = new JvmSystemExiter();
 
 	private static String message = "";

spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/CommandRunner.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2006-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.batch.core.step.tasklet;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * Interface for executing commands. This abstraction is only
+ * useful in order to allow classes that make {@link Runtime#exec} calls
+ * to be testable, since the invoked command might not be
+ * available during tests execution.
+ *
+ * @author Stefano Cordio
+ * @since FIXME
+ */
+public interface CommandRunner {
+
+	/**
+	 * Executes the specified string command in a separate process with the
+	 * specified environment and working directory.
+	 *
+	 * @param   command   a specified system command.
+	 *
+	 * @param   envp      array of strings, each element of which
+	 *                    has environment variable settings in the format
+	 *                    <i>name</i>=<i>value</i>, or
+	 *                    {@code null} if the subprocess should inherit
+	 *                    the environment of the current process.
+	 *
+	 * @param   dir       the working directory of the subprocess, or
+	 *                    {@code null} if the subprocess should inherit
+	 *                    the working directory of the current process.
+	 *
+	 * @return  A new {@link Process} object for managing the subprocess
+	 *
+	 * @throws  SecurityException
+	 *          If a security manager exists and its
+	 *          {@link SecurityManager#checkExec checkExec}
+	 *          method doesn't allow creation of the subprocess
+	 *
+	 * @throws  IOException
+	 *          If an I/O error occurs
+	 *
+	 * @throws  NullPointerException
+	 *          If {@code command} is {@code null},
+	 *          or one of the elements of {@code envp} is {@code null}
+	 *
+	 * @throws  IllegalArgumentException
+	 *          If {@code command} is empty
+	 *
+	 * @see     Runtime#exec(String, String[], File)
+	 */
+	Process exec(String command, String[] envp, File dir) throws IOException;
+
+}

spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/JvmCommandRunner.java

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2006-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.batch.core.step.tasklet;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * Implementation of the {@link CommandRunner} interface that calls the standard
+ * {@link Runtime#exec} method. It should be noted that there is no unit tests for
+ * this class, since there is only one line of actual code, that would only be
+ * testable by mocking {@link Runtime}.
+ *
+ * @author Stefano Cordio
+ * @since FIXME
+ */
+public class JvmCommandRunner implements CommandRunner {
+
+	/**
+	 * Delegate call to {@link Runtime#exec} with the arguments provided.
+	 *
+	 * @see CommandRunner#exec(String, String[], File)
+	 */
+	@Override
+	public Process exec(String command, String[] envp, File dir) throws IOException {
+		return Runtime.getRuntime().exec(command, envp, dir);
+	}
+
+}

spring-batch-core/src/main/java/org/springframework/batch/core/step/tasklet/SystemCommandTasklet.java

@@ -64,6 +64,8 @@ public class SystemCommandTasklet implements StepExecutionListener, StoppableTas
 
 	protected static final Log logger = LogFactory.getLog(SystemCommandTasklet.class);
 
+	private static CommandRunner commandRunner = new JvmCommandRunner();
+
 	private String command;
 
 	private String[] environmentParams = null;
@@ -100,7 +102,7 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkCon
 
 			@Override
 			public Integer call() throws Exception {
-				Process process = Runtime.getRuntime().exec(command, environmentParams, workingDirectory);
+				Process process = commandRunner.exec(command, environmentParams, workingDirectory);
 				return process.waitFor();
 			}
 
@@ -142,6 +144,26 @@ else if (stopped) {
 		}
 	}
 
+	/**
+	 * Static setter for the {@link CommandRunner} so it can be adjusted before
+	 * dependency injection. Typically overridden by
+	 * {@link #setCommandRunner(CommandRunner)}.
+	 *
+	 * @param commandRunner {@link CommandRunner} instance to be used by SystemCommandTasklet instance.
+	 */
+	public static void presetCommandRunner(CommandRunner commandRunner) {
+		SystemCommandTasklet.commandRunner = commandRunner;
+	}
+
+	/**
+	 * Injection setter for the {@link CommandRunner}.
+	 *
+	 * @param commandRunner {@link CommandRunner} instance to be used by SystemCommandTasklet instance.
+	 */
+	public void setCommandRunner(CommandRunner commandRunner) {
+		SystemCommandTasklet.commandRunner = commandRunner;
+	}
+
 	/**
 	 * @param command command to be executed in a separate system process
 	 */
@@ -174,6 +196,7 @@ public void setWorkingDirectory(String dir) {
 
 	@Override
 	public void afterPropertiesSet() throws Exception {
+		Assert.notNull(commandRunner, "CommandRunner must be set");
 		Assert.hasLength(command, "'command' property value is required");
 		Assert.notNull(systemProcessExitCodeMapper, "SystemProcessExitCodeMapper must be set");
 		Assert.isTrue(timeout > 0, "timeout value must be greater than zero");

spring-batch-core/src/test/java/org/springframework/batch/core/step/tasklet/SystemCommandTaskletIntegrationTests.java

@@ -42,6 +42,9 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 /**
@@ -164,6 +167,34 @@ void testInterruption() throws Exception {
 		assertTrue(message.contains(command));
 	}
 
+	/*
+	 * Command Runner is required to be set.
+	 */
+	@Test
+	public void testCommandRunnerNotSet() throws Exception {
+		SystemCommandTasklet.presetCommandRunner(null);
+		try {
+			tasklet.afterPropertiesSet();
+			fail();
+		}
+		catch (IllegalArgumentException e) {
+			// expected
+		} finally {
+			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
+		}
+
+		tasklet.setCommandRunner(null);
+		try {
+			tasklet.afterPropertiesSet();
+			fail();
+		}
+		catch (IllegalArgumentException e) {
+			// expected
+		} finally {
+			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
+		}
+	}
+
 	/*
 	 * Command property value is required to be set.
 	 */
@@ -263,4 +294,52 @@ private boolean isRunningOnWindows() {
 		return System.getProperty("os.name").toLowerCase().contains("win");
 	}
 
+	@Test
+	public void testExecuteWithSuccessfulCommandRunnerMockExecution() throws Exception {
+		try {
+			StepContribution stepContribution = stepExecution.createStepContribution();
+			CommandRunner commandRunner = mock(CommandRunner.class);
+			Process process = mock(Process.class);
+			String command = "invalid command";
+
+			when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
+			when(process.waitFor()).thenReturn(0);
+
+			SystemCommandTasklet.presetCommandRunner(commandRunner);
+			tasklet.setCommand(command);
+			tasklet.afterPropertiesSet();
+
+			RepeatStatus exitStatus = tasklet.execute(stepContribution, null);
+
+			assertEquals(RepeatStatus.FINISHED, exitStatus);
+			assertEquals(ExitStatus.COMPLETED, stepContribution.getExitStatus());
+		} finally {
+			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
+		}
+	}
+
+	@Test
+	public void testExecuteWithFailedCommandRunnerMockExecution() throws Exception {
+		try {
+			StepContribution stepContribution = stepExecution.createStepContribution();
+			CommandRunner commandRunner = mock(CommandRunner.class);
+			Process process = mock(Process.class);
+			String command = "invalid command";
+
+			when(commandRunner.exec(eq(command), any(), any())).thenReturn(process);
+			when(process.waitFor()).thenReturn(1);
+
+			SystemCommandTasklet.presetCommandRunner(commandRunner);
+			tasklet.setCommand(command);
+			tasklet.afterPropertiesSet();
+
+			RepeatStatus exitStatus = tasklet.execute(stepContribution, null);
+
+			assertEquals(RepeatStatus.FINISHED, exitStatus);
+			assertEquals(ExitStatus.FAILED, stepContribution.getExitStatus());
+		} finally {
+			SystemCommandTasklet.presetCommandRunner(new JvmCommandRunner());
+		}
+	}
+
 }