From d5fb4665dca8a0f33634422a5c6fd4ee9582604c Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 23 Feb 2024 11:21:49 -0900 Subject: [PATCH 1/2] Use .bat for Windows version tests --- .../coder/gateway/settings/CoderSettings.kt | 8 +- .../coder/gateway/cli/CoderCLIManagerTest.kt | 146 ++++++++++-------- .../gateway/settings/CoderSettingsTest.kt | 7 + 3 files changed, 95 insertions(+), 66 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt index f0050191..8e52bb28 100644 --- a/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt +++ b/src/main/kotlin/com/coder/gateway/settings/CoderSettings.kt @@ -22,8 +22,10 @@ open class CoderSettings( private val state: CoderSettingsState, // The location of the SSH config. Defaults to ~/.ssh/config. val sshConfigPath: Path = Path.of(System.getProperty("user.home")).resolve(".ssh/config"), - // Env allows overriding the default environment. + // Overrides the default environment (for tests). private val env: Environment = Environment(), + // Overrides the default binary name (for tests). + private val binaryName: String? = null, ) { val tls = CoderTLSSettings(state) val enableDownloads: Boolean @@ -68,10 +70,10 @@ open class CoderSettings( * To where the specified deployment should download the binary. */ fun binPath(url: URL, forceDownloadToData: Boolean = false): Path { - val binaryName = getCoderCLIForOS(getOS(), getArch()) + val name = binaryName ?: getCoderCLIForOS(getOS(), getArch()) val dir = if (forceDownloadToData || state.binaryDirectory.isBlank()) dataDir(url) else withHost(Path.of(expand(state.binaryDirectory)), url) - return dir.resolve(binaryName).toAbsolutePath() + return dir.resolve(name).toAbsolutePath() } /** diff --git a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt index 1b1a29f5..5c2cfe17 100644 --- a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt @@ -32,23 +32,34 @@ import kotlin.test.assertNotEquals import kotlin.test.assertTrue internal class CoderCLIManagerTest { - private fun mkbin(version: String): String { - return listOf("#!/bin/sh", """echo '{"version": "$version"}'""") - .joinToString("\n") + /** + * Return the contents of a script that contains the string. + */ + private fun mkbin(str: String): String { + return if (getOS() == OS.WINDOWS) { + // Must use a .bat extension for this to work. + listOf("@echo off", str) + } else { + listOf("#!/bin/sh", str) + }.joinToString(System.lineSeparator()) + } + + /** + * Return the contents of a script that outputs JSON containing the version. + */ + private fun mkbinVersion(version: String): String { + return mkbin(echo("""{"version": "$version"}""")) } private fun mockServer(errorCode: Int = 0, version: String? = null): Pair { val srv = HttpServer.create(InetSocketAddress(0), 0) srv.createContext("/") {exchange -> var code = HttpURLConnection.HTTP_OK - // TODO: Is there some simple way to create an executable file on - // Windows without having to execute something to generate said - // executable or having to commit one to the repo? - var response = mkbin(version ?: "${srv.address.port}.0.0") + var response = mkbinVersion(version ?: "${srv.address.port}.0.0") val eTags = exchange.requestHeaders["If-None-Match"] if (exchange.requestURI.path == "/bin/override") { code = HttpURLConnection.HTTP_OK - response = mkbin("0.0.0") + response = mkbinVersion("0.0.0") } else if (!exchange.requestURI.path.startsWith("/bin/coder-")) { code = HttpURLConnection.HTTP_NOT_FOUND response = "not found" @@ -159,13 +170,7 @@ internal class CoderCLIManagerTest { assertEquals(true, ccm.download()) - // The mock does not serve a binary that works on Windows so do not - // actually execute. Checking the contents works just as well as proof - // that the binary was correctly downloaded anyway. - assertContains(ccm.localBinaryPath.toFile().readText(), url.port.toString()) - if (getOS() != OS.WINDOWS) { - assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) - } + assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) // It should skip the second attempt. assertEquals(false, ccm.download()) @@ -173,8 +178,7 @@ internal class CoderCLIManagerTest { // Should use the source override. ccm = CoderCLIManager(url, CoderSettings(CoderSettingsState( binarySource = "/bin/override", - dataDirectory = tmpdir.resolve("mock-cli").toString())) - ) + dataDirectory = tmpdir.resolve("mock-cli").toString()))) assertEquals(true, ccm.download()) assertContains(ccm.localBinaryPath.toFile().readText(), "0.0.0") @@ -354,32 +358,52 @@ internal class CoderCLIManagerTest { } } - @Test - fun testFailVersionParse() { - if (getOS() == OS.WINDOWS) { - return // Cannot execute mock binaries on Windows. + /** + * Return an echo command for the OS. + */ + private fun echo(str: String): String { + return if (getOS() == OS.WINDOWS) { + "echo $str" + } else { + "echo '$str'" } + } + /** + * Return an exit command for the OS. + */ + private fun exit(code: Number): String { + return if (getOS() == OS.WINDOWS) { + "exit /b $code" + } else { + "exit $code" + } + } + + @Test + fun testFailVersionParse() { val tests = mapOf( - null to ProcessInitException::class, - """echo '{"foo": true, "baz": 1}'""" to MissingVersionException::class, - """echo '{"version: '""" to JsonSyntaxException::class, - """echo '{"version": "invalid"}'""" to InvalidVersionException::class, - "exit 0" to MissingVersionException::class, - "exit 1" to InvalidExitValueException::class, + null to ProcessInitException::class, + echo("""{"foo": true, "baz": 1}""") to MissingVersionException::class, + echo("""{"version: """) to JsonSyntaxException::class, + echo("""{"version": "invalid"}""") to InvalidVersionException::class, + exit(0) to MissingVersionException::class, + exit(1) to InvalidExitValueException::class, ) val ccm = CoderCLIManager(URL("https://test.coder.parse-fail.invalid"), CoderSettings(CoderSettingsState( - binaryDirectory = tmpdir.resolve("bad-version").toString())) - ) + binaryDirectory = tmpdir.resolve("bad-version").toString()), + binaryName = "coder.bat")) ccm.localBinaryPath.parent.toFile().mkdirs() tests.forEach { if (it.key == null) { ccm.localBinaryPath.toFile().deleteRecursively() } else { - ccm.localBinaryPath.toFile().writeText("#!/bin/sh\n${it.key}") - ccm.localBinaryPath.toFile().setExecutable(true) + ccm.localBinaryPath.toFile().writeText(mkbin(it.key!!)) + if (getOS() != OS.WINDOWS) { + ccm.localBinaryPath.toFile().setExecutable(true) + } } assertFailsWith( exceptionClass = it.value, @@ -389,39 +413,37 @@ internal class CoderCLIManagerTest { @Test fun testMatchesVersion() { - if (getOS() == OS.WINDOWS) { - return - } - val test = listOf( Triple(null, "v1.0.0", null), - Triple("""echo '{"version": "v1.0.0"}'""", "v1.0.0", true), - Triple("""echo '{"version": "v1.0.0"}'""", "v1.0.0-devel+b5b5b5b5", true), - Triple("""echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""", "v1.0.0-devel+b5b5b5b5", true), - Triple("""echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""", "v1.0.0", true), - Triple("""echo '{"version": "v1.0.0-devel+b5b5b5b5"}'""", "v1.0.0-devel+c6c6c6c6", true), - Triple("""echo '{"version": "v1.0.0-prod+b5b5b5b5"}'""", "v1.0.0-devel+b5b5b5b5", true), - Triple("""echo '{"version": "v1.0.0"}'""", "v1.0.1", false), - Triple("""echo '{"version": "v1.0.0"}'""", "v1.1.0", false), - Triple("""echo '{"version": "v1.0.0"}'""", "v2.0.0", false), - Triple("""echo '{"version": "v1.0.0"}'""", "v0.0.0", false), - Triple("""echo '{"version": ""}'""", "v1.0.0", null), - Triple("""echo '{"version": "v1.0.0"}'""", "", null), - Triple("""echo '{"version'""", "v1.0.0", null), - Triple("""exit 0""", "v1.0.0", null), - Triple("""exit 1""", "v1.0.0", null)) + Triple(echo("""{"version": "v1.0.0"}"""), "v1.0.0", true), + Triple(echo("""{"version": "v1.0.0"}"""), "v1.0.0-devel+b5b5b5b5", true), + Triple(echo("""{"version": "v1.0.0-devel+b5b5b5b5"}"""), "v1.0.0-devel+b5b5b5b5", true), + Triple(echo("""{"version": "v1.0.0-devel+b5b5b5b5"}"""), "v1.0.0", true), + Triple(echo("""{"version": "v1.0.0-devel+b5b5b5b5"}"""), "v1.0.0-devel+c6c6c6c6", true), + Triple(echo("""{"version": "v1.0.0-prod+b5b5b5b5"}"""), "v1.0.0-devel+b5b5b5b5", true), + Triple(echo("""{"version": "v1.0.0"}"""), "v1.0.1", false), + Triple(echo("""{"version": "v1.0.0"}"""), "v1.1.0", false), + Triple(echo("""{"version": "v1.0.0"}"""), "v2.0.0", false), + Triple(echo("""{"version": "v1.0.0"}"""), "v0.0.0", false), + Triple(echo("""{"version": ""}"""), "v1.0.0", null), + Triple(echo("""{"version": "v1.0.0"}"""), "", null), + Triple(echo("""{"version"""), "v1.0.0", null), + Triple(exit(0), "v1.0.0", null), + Triple(exit(1), "v1.0.0", null)) val ccm = CoderCLIManager(URL("https://test.coder.matches-version.invalid"), CoderSettings(CoderSettingsState( - binaryDirectory = tmpdir.resolve("matches-version").toString())) - ) + binaryDirectory = tmpdir.resolve("matches-version").toString()), + binaryName = "coder.bat")) ccm.localBinaryPath.parent.toFile().mkdirs() test.forEach { if (it.first == null) { ccm.localBinaryPath.toFile().deleteRecursively() } else { - ccm.localBinaryPath.toFile().writeText("#!/bin/sh\n${it.first}") - ccm.localBinaryPath.toFile().setExecutable(true) + ccm.localBinaryPath.toFile().writeText(mkbin(it.first!!)) + if (getOS() != OS.WINDOWS) { + ccm.localBinaryPath.toFile().setExecutable(true) + } } assertEquals(it.third, ccm.matchesVersion(it.second), it.first) @@ -446,7 +468,9 @@ internal class CoderCLIManagerTest { @Test fun testEnsureCLI() { if (getOS() == OS.WINDOWS) { - return // Cannot execute mock binaries on Windows and setWritable() works differently. + // TODO: setWritable() does not work the same way on Windows but we + // should test what we can. + return } val tests = listOf( @@ -490,7 +514,7 @@ internal class CoderCLIManagerTest { // Create a binary in the regular location. if (it.version != null) { settings.binPath(url).parent.toFile().mkdirs() - settings.binPath(url).toFile().writeText(mkbin(it.version)) + settings.binPath(url).toFile().writeText(mkbinVersion(it.version)) settings.binPath(url).toFile().setExecutable(true) } @@ -503,7 +527,7 @@ internal class CoderCLIManagerTest { // Create a binary in the fallback location. if (it.fallbackVersion != null) { settings.binPath(url, true).parent.toFile().mkdirs() - settings.binPath(url, true).toFile().writeText(mkbin(it.fallbackVersion)) + settings.binPath(url, true).toFile().writeText(mkbinVersion(it.fallbackVersion)) settings.binPath(url, true).toFile().setExecutable(true) } @@ -553,10 +577,6 @@ internal class CoderCLIManagerTest { @Test fun testFeatures() { - if (getOS() == OS.WINDOWS) { - return // Cannot execute mock binaries on Windows. - } - val tests = listOf( Pair("2.5.0", Features(true)), Pair("4.9.0", Features(true)), @@ -567,8 +587,8 @@ internal class CoderCLIManagerTest { tests.forEach { val (srv, url) = mockServer(version = it.first) val ccm = CoderCLIManager(url, CoderSettings(CoderSettingsState( - dataDirectory = tmpdir.resolve("features").toString())) - ) + dataDirectory = tmpdir.resolve("features").toString()), + binaryName = "coder.bat")) assertEquals(true, ccm.download()) assertEquals(it.second, ccm.features, "version: ${it.first}") diff --git a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt index ab1943c6..62cf5ec3 100644 --- a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt @@ -4,6 +4,7 @@ import com.coder.gateway.services.CoderSettingsState import com.coder.gateway.util.OS import com.coder.gateway.util.getOS import com.coder.gateway.util.withPath +import org.junit.Assert.assertNotEquals import java.net.URL import java.nio.file.Path import kotlin.test.Test @@ -84,6 +85,7 @@ internal class CoderSettingsTest { fun testBinPath() { val state = CoderSettingsState() val settings = CoderSettings(state) + val settings2 = CoderSettings(state, binaryName = "foo-bar.baz") // The binary path should fall back to the data directory but that is // already tested in the data directory tests. val url = URL("http://localhost") @@ -92,11 +94,16 @@ internal class CoderSettingsTest { state.binaryDirectory = "/tmp/coder-gateway-test/bin-dir" var expected = "/tmp/coder-gateway-test/bin-dir/localhost" assertEquals(Path.of(expected).toAbsolutePath(), settings.binPath(url).parent) + assertEquals(Path.of(expected).toAbsolutePath(), settings2.binPath(url).parent) // Second argument bypasses override. state.dataDirectory = "/tmp/coder-gateway-test/data-dir" expected = "/tmp/coder-gateway-test/data-dir/localhost" assertEquals(Path.of(expected).toAbsolutePath(), settings.binPath(url, true).parent) + assertEquals(Path.of(expected).toAbsolutePath(), settings2.binPath(url, true).parent) + + assertNotEquals("foo-bar.baz", settings.binPath(url).fileName) + assertEquals("foo-bar.baz", settings2.binPath(url).fileName.toString()) } @Test From 91bcf3ef12863d11f7cc716d90991fb8258a5f8f Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 23 Feb 2024 11:21:56 -0900 Subject: [PATCH 2/2] Clean up some trailing parentheses --- .../coder/gateway/cli/CoderCLIManagerTest.kt | 24 ++++-------- .../gateway/settings/CoderSettingsTest.kt | 39 +++++++------------ 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt index 5c2cfe17..4852d1a2 100644 --- a/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt +++ b/src/test/kotlin/com/coder/gateway/cli/CoderCLIManagerTest.kt @@ -120,8 +120,7 @@ internal class CoderCLIManagerTest { val (srv, url) = mockServer() val ccm = CoderCLIManager(url, CoderSettings(CoderSettingsState( - dataDirectory = tmpdir.resolve("cli-dir-fail-to-write").toString())) - ) + dataDirectory = tmpdir.resolve("cli-dir-fail-to-write").toString()))) ccm.localBinaryPath.parent.toFile().mkdirs() ccm.localBinaryPath.parent.toFile().setWritable(false) @@ -146,8 +145,7 @@ internal class CoderCLIManagerTest { } val ccm = CoderCLIManager(url.toURL(), CoderSettings(CoderSettingsState( - dataDirectory = tmpdir.resolve("real-cli").toString())) - ) + dataDirectory = tmpdir.resolve("real-cli").toString()))) assertTrue(ccm.download()) assertDoesNotThrow { ccm.version() } @@ -165,11 +163,10 @@ internal class CoderCLIManagerTest { fun testDownloadMockCLI() { val (srv, url) = mockServer() var ccm = CoderCLIManager(url, CoderSettings(CoderSettingsState( - dataDirectory = tmpdir.resolve("mock-cli").toString())) - ) + dataDirectory = tmpdir.resolve("mock-cli").toString()), + binaryName = "coder.bat")) assertEquals(true, ccm.download()) - assertEquals(SemVer(url.port.toLong(), 0, 0), ccm.version()) // It should skip the second attempt. @@ -189,8 +186,7 @@ internal class CoderCLIManagerTest { @Test fun testRunNonExistentBinary() { val ccm = CoderCLIManager(URL("https://foo"), CoderSettings(CoderSettingsState( - dataDirectory = tmpdir.resolve("does-not-exist").toString())) - ) + dataDirectory = tmpdir.resolve("does-not-exist").toString()))) assertFailsWith( exceptionClass = ProcessInitException::class, @@ -201,8 +197,7 @@ internal class CoderCLIManagerTest { fun testOverwitesWrongVersion() { val (srv, url) = mockServer() val ccm = CoderCLIManager(url, CoderSettings(CoderSettingsState( - dataDirectory = tmpdir.resolve("overwrite-cli").toString())) - ) + dataDirectory = tmpdir.resolve("overwrite-cli").toString()))) ccm.localBinaryPath.parent.toFile().mkdirs() ccm.localBinaryPath.toFile().writeText("cli") @@ -329,9 +324,7 @@ internal class CoderCLIManagerTest { sshConfigPath = tmpdir.resolve("configured$it.conf")) settings.sshConfigPath.parent.toFile().mkdirs() Path.of("src/test/fixtures/inputs").resolve("$it.conf").toFile().copyTo( - settings.sshConfigPath.toFile(), - true, - ) + settings.sshConfigPath.toFile(), true) val ccm = CoderCLIManager(URL("https://test.coder.invalid"), settings) @@ -349,8 +342,7 @@ internal class CoderCLIManagerTest { tests.forEach { val ccm = CoderCLIManager(URL("https://test.coder.invalid"), CoderSettings(CoderSettingsState( - headerCommand = it)) - ) + headerCommand = it))) assertFailsWith( exceptionClass = Exception::class, diff --git a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt index 62cf5ec3..fcc50799 100644 --- a/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt +++ b/src/test/kotlin/com/coder/gateway/settings/CoderSettingsTest.kt @@ -36,8 +36,7 @@ internal class CoderSettingsTest { env = Environment(mapOf( "LOCALAPPDATA" to "/tmp/coder-gateway-test/localappdata", "HOME" to "/tmp/coder-gateway-test/home", - "XDG_DATA_HOME" to "/tmp/coder-gateway-test/xdg-data")) - ) + "XDG_DATA_HOME" to "/tmp/coder-gateway-test/xdg-data"))) var expected = when(getOS()) { OS.WINDOWS -> "/tmp/coder-gateway-test/localappdata/coder-gateway/localhost" OS.MAC -> "/tmp/coder-gateway-test/home/Library/Application Support/coder-gateway/localhost" @@ -52,8 +51,7 @@ internal class CoderSettingsTest { settings = CoderSettings(state, env = Environment(mapOf( "XDG_DATA_HOME" to "", - "HOME" to "/tmp/coder-gateway-test/home")) - ) + "HOME" to "/tmp/coder-gateway-test/home"))) expected = "/tmp/coder-gateway-test/home/.local/share/coder-gateway/localhost" assertEquals(Path.of(expected).toAbsolutePath(), settings.dataDir(url)) @@ -66,8 +64,7 @@ internal class CoderSettingsTest { env = Environment(mapOf( "LOCALAPPDATA" to "/ignore", "HOME" to "/ignore", - "XDG_DATA_HOME" to "/ignore")) - ) + "XDG_DATA_HOME" to "/ignore"))) expected = "/tmp/coder-gateway-test/data-dir/localhost" assertEquals(Path.of(expected).toAbsolutePath(), settings.dataDir(url)) assertEquals(Path.of(expected).toAbsolutePath(), settings.binPath(url).parent) @@ -110,11 +107,10 @@ internal class CoderSettingsTest { fun testCoderConfigDir() { val state = CoderSettingsState() var settings = CoderSettings(state, - env = Environment( - mapOf("APPDATA" to "/tmp/coder-gateway-test/cli-appdata", - "HOME" to "/tmp/coder-gateway-test/cli-home", - "XDG_CONFIG_HOME" to "/tmp/coder-gateway-test/cli-xdg-config")) - ) + env = Environment(mapOf( + "APPDATA" to "/tmp/coder-gateway-test/cli-appdata", + "HOME" to "/tmp/coder-gateway-test/cli-home", + "XDG_CONFIG_HOME" to "/tmp/coder-gateway-test/cli-xdg-config"))) var expected = when(getOS()) { OS.WINDOWS -> "/tmp/coder-gateway-test/cli-appdata/coderv2" OS.MAC -> "/tmp/coder-gateway-test/cli-home/Library/Application Support/coderv2" @@ -125,25 +121,20 @@ internal class CoderSettingsTest { // Fall back to HOME on Linux. if (getOS() == OS.LINUX) { settings = CoderSettings(state, - env = Environment( - mapOf("XDG_CONFIG_HOME" to "", - "HOME" to "/tmp/coder-gateway-test/cli-home", - )) - ) + env = Environment(mapOf( + "XDG_CONFIG_HOME" to "", + "HOME" to "/tmp/coder-gateway-test/cli-home"))) expected = "/tmp/coder-gateway-test/cli-home/.config/coderv2" assertEquals(Path.of(expected), settings.coderConfigDir) } // Read CODER_CONFIG_DIR. settings = CoderSettings(state, - env = Environment( - mapOf( - "CODER_CONFIG_DIR" to "/tmp/coder-gateway-test/coder-config-dir", - "APPDATA" to "/ignore", - "HOME" to "/ignore", - "XDG_CONFIG_HOME" to "/ignore", - )) - ) + env = Environment(mapOf( + "CODER_CONFIG_DIR" to "/tmp/coder-gateway-test/coder-config-dir", + "APPDATA" to "/ignore", + "HOME" to "/ignore", + "XDG_CONFIG_HOME" to "/ignore"))) expected = "/tmp/coder-gateway-test/coder-config-dir" assertEquals(Path.of(expected), settings.coderConfigDir) }