Fix docker_id variation when using custom docker registry

CHANGELOG.md

@@ -16,6 +16,7 @@
 
 * `resource path`: Don't finalize the `path` field of a resource until it's written as part of building a component (PR #668).
 
+* `viash test`: Fix Docker id between built 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
 

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

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)

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.