diff --git a/CHANGELOG.md b/CHANGELOG.md index 14f928149..f3a952232 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ TODO add summary * `requirements`: Improve the error message when a Python or R requirement uses a single quote in the `.script` field (PR #675). +* `viash test`: Fix Docker id between build and test components not being consistent when using a custom Docker registry (PR #679). + # Viash 0.9.0-RC2 (2024-02-23): Restructure the config and change some default values The `.functionality` layer has been removed from the config and all fields have been moved to the top layer. diff --git a/src/main/scala/io/viash/ViashTest.scala b/src/main/scala/io/viash/ViashTest.scala index e054a1706..cf62cd5f2 100644 --- a/src/main/scala/io/viash/ViashTest.scala +++ b/src/main/scala/io/viash/ViashTest.scala @@ -263,7 +263,10 @@ object ViashTest extends Logging { // set dirArg as argument so that Docker can chown it after execution argument_groups = List(ArgumentGroup("default", None, List(dirArg))), resources = List(test), - set_wd_to_resources_dir = true + set_wd_to_resources_dir = true, + // Make sure we'll be using the same docker registry set in 'links' so we can have the same docker image id. + // Copy the whole case class instead of selective copy. + links = conf.links, ))(appliedConfig) // generate bash script for test diff --git a/src/test/scala/io/viash/e2e/build/DockerMoreSuite.scala b/src/test/scala/io/viash/e2e/build/DockerMoreSuite.scala index a8ff6e02c..e768fcb09 100644 --- a/src/test/scala/io/viash/e2e/build/DockerMoreSuite.scala +++ b/src/test/scala/io/viash/e2e/build/DockerMoreSuite.scala @@ -4,7 +4,7 @@ import io.viash._ import org.scalatest.BeforeAndAfterAll import org.scalatest.funsuite.AnyFunSuite -import java.nio.file.{Files, Paths, StandardCopyOption} +import java.nio.file.{Files, Paths, StandardCopyOption, Path} import io.viash.helpers.{IO, Exec, Logger} import io.viash.config.Config @@ -133,6 +133,65 @@ class DockerMoreSuite extends AnyFunSuite with BeforeAndAfterAll { assert(stdout.contains("""real_number: |123.456;123;789|""")) } + test("Check docker image id", DockerTest) { + val newConfigFilePath = configDeriver.derive(Nil, "build_docker_id") + val testText = TestHelper.testMain( + "build", + "--engine", "docker", + "--runner", "executable", + "-o", tempFolStr, + newConfigFilePath, + "--setup", "alwaysbuild" + ) + + assert(executable.exists) + assert(executable.canExecute) + + // read built files and get docker image ids + def getDockerId(builtScriptPath: Path): Option[String] = { + val dockerImageIdRegex = ".*\\sVIASH_DOCKER_IMAGE_ID='(.[^']*)'.*".r + val content = IO.read(builtScriptPath.toUri()) + + content.replaceAll("\n", "") match { + case dockerImageIdRegex(id) => Some(id) + case _ => None + } + } + + val dockerId = getDockerId(executable.toPath()) + + assert(dockerId == Some("testbash:0.1")) + } + + test("Check docker image id with custom docker_registry", DockerTest) { + val newConfigFilePath = configDeriver.derive(""".links := {docker_registry: "foo.bar"}""", "build_docker_id_custom_registry") + val testText = TestHelper.testMain( + "build", + "--engine", "docker", + "--runner", "executable", + "-o", tempFolStr, + newConfigFilePath, + "--setup", "alwaysbuild" + ) + + assert(executable.exists) + assert(executable.canExecute) + + // read built files and get docker image ids + def getDockerId(builtScriptPath: Path): Option[String] = { + val dockerImageIdRegex = ".*\\sVIASH_DOCKER_IMAGE_ID='(.[^']*)'.*".r + val content = IO.read(builtScriptPath.toUri()) + + content.replaceAll("\n", "") match { + case dockerImageIdRegex(id) => Some(id) + case _ => None + } + } + + val dockerId = getDockerId(executable.toPath()) + + assert(dockerId == Some("foo.bar/testbash:0.1")) + } override def afterAll(): Unit = { IO.deleteRecursively(temporaryFolder) diff --git a/src/test/scala/io/viash/e2e/test/MainTestDockerSuite.scala b/src/test/scala/io/viash/e2e/test/MainTestDockerSuite.scala index 1a68bbe90..44ffd466b 100644 --- a/src/test/scala/io/viash/e2e/test/MainTestDockerSuite.scala +++ b/src/test/scala/io/viash/e2e/test/MainTestDockerSuite.scala @@ -11,6 +11,7 @@ import org.scalatest.funsuite.AnyFunSuite import scala.reflect.io.Directory import sys.process._ import org.scalatest.ParallelTestExecution +import java.nio.file.Path class MainTestDockerSuite extends AnyFunSuite with BeforeAndAfterAll with ParallelTestExecution{ Logger.UseColorOverride.value = Some(false) @@ -226,6 +227,104 @@ class MainTestDockerSuite extends AnyFunSuite with BeforeAndAfterAll with Parall checkTempDirAndRemove(testText.stdout, false) } + test("Check docker image id", DockerTest) { + val newConfigFilePath = configDeriver.derive(Nil, "test_docker_id") + val testText = TestHelper.testMain( + "test", + "--engine", "docker", + "--keep", "true", + newConfigFilePath + ) + + assert(testText.stdout.contains("Running tests in temporary directory: ")) + assert(testText.stdout.contains("SUCCESS! All 2 out of 2 test scripts succeeded!")) + assert(!testText.stdout.contains("Cleaning up temporary directory")) + + // Getting the temp folder path, copied from 'checkTempDirAndRemove'. It works \o/ + val FolderRegex = ".*Running tests in temporary directory: '([^']*)'.*".r + + val tempPath = testText.stdout.replaceAll("\n", "") match { + case FolderRegex(path) => path + case _ => "" + } + + assert(tempPath.contains(s"${IO.tempDir}/viash_test_testbash")) + + // check the expected files exist + val buildEngineEnvironmentPath = Paths.get(tempPath, "build_engine_environment/testbash") + val testScriptPath = Paths.get(tempPath, "test_check_outputs/test_executable") + assert(buildEngineEnvironmentPath.toFile.exists) + assert(testScriptPath.toFile.exists) + + // read built files and get docker image ids + def getDockerId(builtScriptPath: Path): Option[String] = { + val dockerImageIdRegex = ".*\\sVIASH_DOCKER_IMAGE_ID='(.[^']*)'.*".r + val content = IO.read(builtScriptPath.toUri()) + + content.replaceAll("\n", "") match { + case dockerImageIdRegex(id) => Some(id) + case _ => None + } + } + + val buildId = getDockerId(buildEngineEnvironmentPath) + val testId = getDockerId(testScriptPath) + + assert(buildId == Some("testbash:test")) + assert(testId == Some("testbash:test")) + + checkTempDirAndRemove(testText.stdout, true) + } + + test("Check docker image id with custom docker_registry", DockerTest) { + val newConfigFilePath = configDeriver.derive(""".links := {docker_registry: "foo.bar"}""", "test_docker_id_custom_registry") + val testText = TestHelper.testMain( + "test", + "--engine", "docker", + "--keep", "true", + newConfigFilePath + ) + + assert(testText.stdout.contains("Running tests in temporary directory: ")) + assert(testText.stdout.contains("SUCCESS! All 2 out of 2 test scripts succeeded!")) + assert(!testText.stdout.contains("Cleaning up temporary directory")) + + // Getting the temp folder path, copied from 'checkTempDirAndRemove'. It works \o/ + val FolderRegex = ".*Running tests in temporary directory: '([^']*)'.*".r + + val tempPath = testText.stdout.replaceAll("\n", "") match { + case FolderRegex(path) => path + case _ => "" + } + + assert(tempPath.contains(s"${IO.tempDir}/viash_test_testbash")) + + // check the expected files exist + val buildEngineEnvironmentPath = Paths.get(tempPath, "build_engine_environment/testbash") + val testScriptPath = Paths.get(tempPath, "test_check_outputs/test_executable") + assert(buildEngineEnvironmentPath.toFile.exists) + assert(testScriptPath.toFile.exists) + + // read built files and get docker image ids + def getDockerId(builtScriptPath: Path): Option[String] = { + val dockerImageIdRegex = ".*\\sVIASH_DOCKER_IMAGE_ID='(.[^']*)'.*".r + val content = IO.read(builtScriptPath.toUri()) + + content.replaceAll("\n", "") match { + case dockerImageIdRegex(id) => Some(id) + case _ => None + } + } + + val buildId = getDockerId(buildEngineEnvironmentPath) + val testId = getDockerId(testScriptPath) + + assert(buildId == Some("foo.bar/testbash:test")) + assert(testId == Some("foo.bar/testbash:test")) + + checkTempDirAndRemove(testText.stdout, true) + } + /** * Searches the output generated by Main.main() during tests for the temporary directory name and verifies if it still exists or not. * If directory was expected to be present and actually is present, it will be removed.