From bf3755732889a6e1c915641d7b3d50765ce9f719 Mon Sep 17 00:00:00 2001 From: Piotr Adamczyk Date: Mon, 9 Aug 2021 08:15:39 +0200 Subject: [PATCH] refactor: Bash command execution --- .../main/kotlin/ftl/adapter/CommandRunner.kt | 10 ++++ test_runner/src/main/kotlin/ftl/api/Bash.kt | 12 +++++ .../main/kotlin/ftl/client/RunShellCommand.kt | 50 +++++++++++++++++++ .../ftl/ios/xctest/common/ParseObjTests.kt | 17 ++++--- .../ftl/ios/xctest/common/ParseSwiftTests.kt | 15 +++--- .../src/main/kotlin/ftl/mock/MockServer.kt | 7 +-- test_runner/src/main/kotlin/ftl/util/Bash.kt | 45 ----------------- .../src/test/kotlin/ftl/util/BashTest.kt | 21 ++++---- 8 files changed, 106 insertions(+), 71 deletions(-) create mode 100644 test_runner/src/main/kotlin/ftl/adapter/CommandRunner.kt create mode 100644 test_runner/src/main/kotlin/ftl/api/Bash.kt create mode 100644 test_runner/src/main/kotlin/ftl/client/RunShellCommand.kt delete mode 100644 test_runner/src/main/kotlin/ftl/util/Bash.kt diff --git a/test_runner/src/main/kotlin/ftl/adapter/CommandRunner.kt b/test_runner/src/main/kotlin/ftl/adapter/CommandRunner.kt new file mode 100644 index 0000000000..4685047b7e --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/adapter/CommandRunner.kt @@ -0,0 +1,10 @@ +package ftl.adapter + +import ftl.api.Command +import ftl.client.runShellCommand + +object CommandRunner : + Command.Fetch, + (Command) -> String by { (cmd, additionalPaths) -> + runShellCommand(cmd, additionalPaths) + } diff --git a/test_runner/src/main/kotlin/ftl/api/Bash.kt b/test_runner/src/main/kotlin/ftl/api/Bash.kt new file mode 100644 index 0000000000..03aef36eb0 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/api/Bash.kt @@ -0,0 +1,12 @@ +package ftl.api + +import ftl.adapter.CommandRunner + +val runCommand: Command.Fetch get() = CommandRunner + +data class Command( + val cmd: String, + val additionalPath: List> = emptyList() +) { + interface Fetch : (Command) -> String +} diff --git a/test_runner/src/main/kotlin/ftl/client/RunShellCommand.kt b/test_runner/src/main/kotlin/ftl/client/RunShellCommand.kt new file mode 100644 index 0000000000..fd41a4faf7 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/client/RunShellCommand.kt @@ -0,0 +1,50 @@ +package ftl.client + +import flank.common.isWindows +import ftl.run.exception.FlankGeneralError +import ftl.util.failed +import ftl.util.waitForResult + +sealed class ShellEnvironment(val execution: String) { + object Bash : ShellEnvironment("Bash.exe") + object BinBash : ShellEnvironment("/bin/bash") + object Cmd : ShellEnvironment("cmd.exe") + object Default : ShellEnvironment(if (isWindows) "cmd.exe" else "/bin/bash") +} + +fun runShellCommand( + cmd: String, + additionalPath: List> +): String { + val process = ProcessBuilder(shell.execution, command, cmd) + .appendAdditionalPaths(additionalPath) + .redirectOutput(ProcessBuilder.Redirect.PIPE) + .redirectError(ProcessBuilder.Redirect.PIPE) + .start() + + val result = process.waitForResult() + + if (process.failed()) { + System.err.println("Error: ${result.stderr}") + throw FlankGeneralError("Command failed: $cmd") + } + + return result.stdout.trim() + result.stderr.trim() +} + +private fun ProcessBuilder.appendAdditionalPaths( + additionalPath: List> +) = apply { + val envs: MutableMap = environment() + additionalPath.onEach { (key, value) -> envs[key] = value } +} + +private val shell: ShellEnvironment by lazy { + when { + isWindows -> ShellEnvironment.Cmd + else -> ShellEnvironment.BinBash + } +} + +private val command: String + get() = if (isWindows) "/c" else "-c" diff --git a/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseObjTests.kt b/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseObjTests.kt index f3b0c39b8e..ef29c9943b 100644 --- a/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseObjTests.kt +++ b/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseObjTests.kt @@ -3,7 +3,8 @@ package ftl.ios.xctest.common import flank.common.appDataDirectory import flank.common.isMacOS import flank.common.isWindows -import ftl.util.Bash +import ftl.api.Command +import ftl.api.runCommand internal fun parseObjcTests(binary: String): List { installBinaries @@ -15,12 +16,16 @@ internal fun parseObjcTests(binary: String): List { var cmd = if (!isWindows) "nm -U ${binary.quote()}" else "llvm-nm.exe -U ${binary.quote()}" if (!isMacOS) cmd = if (isWindows) cmd.replace("\\", "/") else "PATH=~/.flank $cmd" - val path = if (isWindows) listOf(Pair("Path", "$appDataDirectory\\.flank\\;C:\\Windows\\System32\\")) - else emptyList() - - val output = Bash.execute(cmd, path) + val command = Command( + cmd = cmd, + additionalPath = if (isWindows) { + listOf(Pair("Path", "$appDataDirectory\\.flank\\;C:\\Windows\\System32\\")) + } else { + emptyList() + } + ) - output.lines().forEach { line -> + runCommand(command).lines().forEach { line -> // 000089b0 t -[EarlGreyExampleTests testLayout] // 00008330 t -[EarlGreyExampleTests testCustomAction] val pattern = """.+\st\s-\[(.+\stest.+)]""".toRegex() diff --git a/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseSwiftTests.kt b/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseSwiftTests.kt index 03ac6b7b3d..aed89a5915 100644 --- a/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseSwiftTests.kt +++ b/test_runner/src/main/kotlin/ftl/ios/xctest/common/ParseSwiftTests.kt @@ -4,8 +4,8 @@ import flank.common.appDataDirectory import flank.common.isLinux import flank.common.isMacOS import flank.common.isWindows -import ftl.util.Bash -import ftl.util.ShellEnvironment +import ftl.api.Command +import ftl.api.runCommand internal fun parseSwiftTests(binary: String): List { installBinaries @@ -25,14 +25,13 @@ internal fun parseSwiftTests(binary: String): List { } val path = if (isWindows) { - listOf(Pair("Path", "$appDataDirectory\\.flank\\;C:\\Windows\\System32\\"), Pair("LD_LIBRARY_PATH", "$appDataDirectory\\.flank\\")) + listOf( + Pair("Path", "$appDataDirectory\\.flank\\;C:\\Windows\\System32\\"), + Pair("LD_LIBRARY_PATH", "$appDataDirectory\\.flank\\") + ) } else emptyList() - // https://github.com/linkedin/bluepill/blob/37e7efa42472222b81adaa0e88f2bd82aa289b44/Source/Shared/BPXCTestFile.m#L17-18 - val shell = if (isWindows) ShellEnvironment.Cmd else ShellEnvironment.Default - - val demangledOutput = Bash.execute(cmd, path, shell) - demangledOutput.lines().forEach { line -> + runCommand(Command(cmd, path)).lines().forEach { line -> // _T025EarlGreyExampleTestsSwift0abceD0C10testLayoutyyF ---> EarlGreyExampleTestsSwift.EarlGreyExampleSwiftTests.testLayout() -> () // _T025EarlGreyExampleTestsSwift0abceD0C16testCustomActionyyF ---> EarlGreyExampleTestsSwift.EarlGreyExampleSwiftTests.testCustomAction() -> () // _$S25EarlGreyExampleTestsSwift0abceD0C14testThatThrowsyyKF ---> EarlGreyExampleTestsSwift.EarlGreyExampleSwiftTests.testThatThrows() throws -> () diff --git a/test_runner/src/main/kotlin/ftl/mock/MockServer.kt b/test_runner/src/main/kotlin/ftl/mock/MockServer.kt index de6a0c4064..9a5bab1ef9 100644 --- a/test_runner/src/main/kotlin/ftl/mock/MockServer.kt +++ b/test_runner/src/main/kotlin/ftl/mock/MockServer.kt @@ -44,12 +44,13 @@ import com.google.testing.model.TestExecution import com.google.testing.model.TestMatrix import com.google.testing.model.ToolResultsExecution import com.google.testing.model.ToolResultsStep +import ftl.api.Command +import ftl.api.runCommand import ftl.client.google.run.toClientInfoDetailList import ftl.config.FtlConstants import ftl.config.FtlConstants.JSON_FACTORY import ftl.log.LogbackLogger import ftl.run.exception.FlankGeneralError -import ftl.util.Bash import ftl.util.StepOutcome.failure import ftl.util.StepOutcome.flaky import ftl.util.StepOutcome.inconclusive @@ -420,9 +421,9 @@ object MockServer { try { server.start(wait = false) } catch (e: BindException) { - val lsofOutput = Bash.execute("lsof -i :$port") + val lsofOutput = runCommand(Command("lsof -i :$port")) val pid = lsofOutput.split("\n").last().split(Regex("\\s+"))[1] - Bash.execute("kill -9 $pid") + runCommand(Command("kill -9 $pid")) Thread.sleep(2000) server.start(wait = false) } diff --git a/test_runner/src/main/kotlin/ftl/util/Bash.kt b/test_runner/src/main/kotlin/ftl/util/Bash.kt deleted file mode 100644 index 6b82eff1b1..0000000000 --- a/test_runner/src/main/kotlin/ftl/util/Bash.kt +++ /dev/null @@ -1,45 +0,0 @@ -package ftl.util - -import flank.common.isWindows -import flank.common.logLn -import ftl.run.exception.FlankGeneralError -import java.lang.ProcessBuilder.Redirect.PIPE - -object Bash { - fun execute( - cmd: String, - additionalPath: List> = emptyList(), - shellEnvironment: ShellEnvironment = ShellEnvironment.Default - ): String { - logLn(cmd) - val process = ProcessBuilder( - shellEnvironment.execution, - if (isWindows) "/c" - else "-c", - cmd - ).also { - if (additionalPath.isNotEmpty()) { - val envs: MutableMap = it.environment() - additionalPath.forEach { extra -> - envs[extra.first] = extra.second - } - } - }.redirectOutput(PIPE).redirectError(PIPE).start() - - val result = process.waitForResult() - - if (process.failed()) { - System.err.println("Error: ${result.stderr}") - throw FlankGeneralError("Command failed: $cmd") - } - - return result.stdout.trim() + result.stderr.trim() - } -} - -sealed class ShellEnvironment(val execution: String) { - object Bash : ShellEnvironment("Bash.exe") - object BinBash : ShellEnvironment("/bin/bash") - object Cmd : ShellEnvironment("cmd.exe") - object Default : ShellEnvironment(if (isWindows) "cmd.exe" else "/bin/bash") -} diff --git a/test_runner/src/test/kotlin/ftl/util/BashTest.kt b/test_runner/src/test/kotlin/ftl/util/BashTest.kt index 5824b4c8f6..1ca7d28d2b 100644 --- a/test_runner/src/test/kotlin/ftl/util/BashTest.kt +++ b/test_runner/src/test/kotlin/ftl/util/BashTest.kt @@ -3,8 +3,11 @@ package ftl.util import com.google.common.truth.Truth.assertThat import flank.common.isMacOS import flank.common.isWindows +import ftl.api.Command +import ftl.api.runCommand import ftl.run.exception.FlankGeneralError import ftl.test.util.FlankTestRunner +import org.junit.Assume.assumeFalse import org.junit.Test import org.junit.runner.RunWith import java.io.File @@ -14,31 +17,31 @@ class BashTest { @Test fun executeStderr() { - if (isWindows) return - assertThat(Bash.execute("echo a 1>&2")).isEqualTo("a") + assumeFalse(isWindows) + assertThat(runCommand(Command("echo a 1>&2"))).isEqualTo("a") } @Test(expected = FlankGeneralError::class) fun executeStderrExitCode1() { if (isWindows) throw FlankGeneralError("Failure as is windows") - assertThat(Bash.execute("echo not an error 1>&2; exit 1")).isEmpty() + assertThat(runCommand(Command("echo not an error 1>&2; exit 1"))).isEmpty() } @Test fun executeNoOutput() { - if (isWindows) return - assertThat(Bash.execute(" ")).isEmpty() + assumeFalse(isWindows) + assertThat(runCommand(Command(" "))).isEmpty() } @Test fun executeSmallOutput() { - if (isWindows) return - assertThat(Bash.execute("echo ok")).isEqualTo("ok") + assumeFalse(isWindows) + assertThat(runCommand(Command("echo ok"))).isEqualTo("ok") } @Test fun executeLargeOutput() { - if (isWindows) return + assumeFalse(isWindows) // gohello is a binary that outputs 100k 'hi' to stdout val os = if (isMacOS) { "mac" @@ -48,6 +51,6 @@ class BashTest { val cmd = "./src/test/kotlin/ftl/fixtures/tmp/gohello/bin/$os/gohello" // ensure binary is marked executable. unzip may have removed the executable bit. File(cmd).setExecutable(true) - assertThat(Bash.execute(cmd).length).isEqualTo(200_000) + assertThat(runCommand(Command(cmd)).length).isEqualTo(200_000) } }