R dockerized test run gradle target and CI (deephaven#4625)

* Draft.

* Shaping up; cpp-client build image step now works; R test run fails but containers created.

* Almost there... missing xml2 package in R for junit reporter.

* Update gradle.properties

* Updated r-client-base tag.

* Fix issues with right image.

* Make R tests configurable for host:port.

* Moving away from MAKEFLAGS into MAKE env var for R builds

* Now it works

* New r-client-base image.

* Refactored the build code, separated image building, handle errors inside R with proper exit status.

* R's install.packages only gives a warning on compilation failure... sigh.

* Followup to James/Colin review comments on dependsOn.

* Followup to the followup to the followup to review comments (ref dependsOn).

* Followup to review comments.

* Followup to review comments: remove references to cpp-client-base from deephaven-core repo.

.github/workflows/tag-base-images.yml

@@ -59,7 +59,7 @@ jobs:
       - name: Tag upstream images
         if: ${{ startsWith(github.ref, 'refs/heads/release/v') }}
         run: |
-          ./docker/registry/cpp-client-base/build/crane/retag.sh
+          ./docker/registry/r-client-base/build/crane/retag.sh
           ./docker/registry/protoc-base/build/crane/retag.sh
           ./docker/registry/slim-base/build/crane/retag.sh
           ./docker/registry/server-base/build/crane/retag.sh

R/build.gradle

@@ -0,0 +1,111 @@
+plugins {
+    id 'com.bmuschko.docker-remote-api'
+    id 'io.deephaven.project.register'
+    id 'io.deephaven.deephaven-in-docker'
+}
+
+evaluationDependsOn(':cpp-client')
+
+configurations {
+    cpp {}
+}
+
+dependencies {
+  cpp project(':cpp-client')
+}
+
+def prefix = '/opt/deephaven'
+
+// start a grpc-api server
+String randomSuffix = UUID.randomUUID().toString();
+deephavenDocker {
+    envVars.set([
+            'START_OPTS':'-Xmx512m -DAuthHandlers=io.deephaven.auth.AnonymousAuthenticationHandler'
+    ])
+    containerName.set "dh-server-for-r-${randomSuffix}"
+    networkName.set "r-test-network-${randomSuffix}"
+}
+
+def buildRClient = Docker.registerDockerTask(project, 'rClient') {
+    // Only tested on x86-64, and we only build dependencies for x86-64
+    platform = 'linux/amd64'
+
+    copyIn {
+        from(layout.projectDirectory) {
+            include 'r-build.sh'
+            include 'r-tests.sh'
+            include 'rdeephaven/DESCRIPTION'
+            include 'rdeephaven/LICENSE'
+            include 'rdeephaven/NAMESPACE'
+            include 'rdeephaven/README.md'
+            include 'rdeephaven/inst/**'
+            include 'rdeephaven/man/**'
+            include 'rdeephaven/etc/**'
+            include 'rdeephaven/R/**'
+            include 'rdeephaven/src/*.cpp'
+            include 'rdeephaven/src/Makevars'
+        }
+    }
+    dockerfile {
+        from('deephaven/cpp-client:local-build')
+        //
+        // Build and install client.
+        //
+        runCommand("""mkdir -p \\
+                        /out \\
+                        ${prefix}/log \\
+                        ${prefix}/bin/rdeephaven \\
+                        ${prefix}/src/rdeephaven/{inst,man,etc,src,R,bin}
+                   """)
+        copyFile('rdeephaven/DESCRIPTION',  "${prefix}/src/rdeephaven/")
+        copyFile('rdeephaven/LICENSE',  "${prefix}/src/rdeephaven/")
+        copyFile('rdeephaven/NAMESPACE',  "${prefix}/src/rdeephaven/")
+        copyFile('rdeephaven/README.md',  "${prefix}/src/rdeephaven/")
+        copyFile('rdeephaven/inst/',  "${prefix}/src/rdeephaven/inst/")
+        copyFile('rdeephaven/man/', "${prefix}/src/rdeephaven/man/")
+        copyFile('rdeephaven/etc/', "${prefix}/src/rdeephaven/etc/")
+        copyFile('rdeephaven/R/', "${prefix}/src/rdeephaven/R/")
+        copyFile('rdeephaven/src/*.cpp', "${prefix}/src/rdeephaven/src/")
+        copyFile('rdeephaven/src/Makevars', "${prefix}/src/rdeephaven/src/")
+        copyFile('r-tests.sh', "${prefix}/bin/rdeephaven")
+        copyFile('r-build.sh', "${prefix}/bin/rdeephaven")
+        runCommand("PREFIX=${prefix}; " +
+                  '''set -eux ; \
+                     cd "${PREFIX}/src/rdeephaven"; \
+                     . "${PREFIX}/env.sh"; \
+                     ${PREFIX}/bin/rdeephaven/r-build.sh
+                   ''')
+    }
+    parentContainers = [ project.tasks.getByPath(':cpp-client:cppClient') ]
+}
+
+def testRClient = Docker.registerDockerTask(project, 'testRClient') {
+    // Only tested on x86-64, and we only build dependencies for x86-64
+    platform = 'linux/amd64'
+    copyIn {
+        from(layout.projectDirectory) {
+            include 'r-tests.sh'
+            include 'rdeephaven/inst/**'
+        }
+    }
+    copyOut {
+        into layout.buildDirectory.dir('test-results')
+    }
+    dockerfile {
+        from('deephaven/r-client:local-build')
+        //
+        // Setup for test run; we should be inheriting other env vars
+        // like LD_LIBRARY_PATH from the cpp-client image.
+        //
+        environmentVariable 'DH_HOST', deephavenDocker.containerName.get()
+        environmentVariable 'DH_PORT', '10000'
+    }
+    containerDependencies.dependsOn = [deephavenDocker.healthyTask]
+    containerDependencies.finalizedBy = deephavenDocker.endTask
+    network = deephavenDocker.networkName.get()
+    parentContainers = [ project.tasks.getByName('rClient') ]
+    entrypoint = ["${prefix}/bin/rdeephaven/r-tests.sh", '/out/r-test.xml', '/out/r-test.log']
+}
+
+deephavenDocker.shouldLogIfTaskFails testRClient
+tasks.check.dependsOn(testRClient)

R/gradle.properties

@@ -0,0 +1 @@
+io.deephaven.project.ProjectType=BASIC

R/r-build.sh

@@ -0,0 +1,36 @@
+#!/bin/bash
+
+set -euo pipefail
+
+for var in DHCPP NCPUS LD_LIBRARY_PATH; do
+    if [ -z "${!var}" ]; then
+        echo "$0: Environment variable $var is not set, aborting." 1>&2
+        exit 1
+    fi
+done
+
+if [ ! -d ./src ] || [ ! -f ./src/Makevars ] || [ ! -d ./R ] || [ ! -f ./DESCRIPTION ]; then
+    echo "The current directory `pwd` does not look like an R package source directory, aborting." 1>&2
+    exit 1
+fi
+
+# Ensure builds are always done from a clean slate.
+trap 'rm -f src/*.o src/*.so' 1 2 15
+rm -f src/*.o src/*.so
+
+MAKE="make -j${NCPUS}" R --no-save --no-restore <<EOF
+status = tryCatch(
+  {
+     install.packages(".", repos=NULL, type="source")
+     0
+  },
+  error=function(e) 1,
+  warning=function(w) 2
+)
+print(paste0('status=', status))
+quit(save='no', status=status)
+EOF
+
+rm -f src/*.o src/*.so
+
+exit 0

R/r-tests.sh

@@ -0,0 +1,34 @@
+#!/bin/bash
+
+set -euo pipefail
+
+if [ "$#" -ne 2 ]; then
+    echo "Usage: $0 out.xml out.log" 1>&2
+    exit 1
+fi
+
+if [ -z "${DH_PREFIX}" ]; then
+    echo "$0: Environment variable DH_PREFIX is not set, aborting." 1>&2
+    exit 1
+fi
+
+cd $DH_PREFIX/src/rdeephaven
+
+OUT_XML="$1"
+OUT_LOG="$2"
+
+R --no-save --no-restore <<EOF >& "${OUT_LOG}"
+library('testthat')
+options(testthat.output_file = '${OUT_XML}')
+status = tryCatch(
+  {
+     test_package('rdeephaven', reporter = 'junit')
+     0
+  },
+  error=function(e) 1
+)
+print(paste0('status=', status))
+quit(save='no', status=status)
+EOF
+
+exit 0

R/rdeephaven/inst/tests/testthat/helper.R

@@ -0,0 +1,11 @@
+get_dh_target <- function() {
+  dh_host = Sys.getenv("DH_HOST")
+  if (dh_host == '') {
+    dh_host = "localhost"
+  }
+  dh_port = Sys.getenv("DH_PORT")
+  if (dh_port == '') {
+    dh_port = 10000
+  }
+  return(paste0(dh_host, ':', dh_port))
+}

R/rdeephaven/inst/tests/testthat/test_agg_by.R

@@ -38,7 +38,7 @@ setup <- function() {
   )
 
   # set up client
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = get_dh_target())
 
   # move dataframes to server and get TableHandles for testing
   th1 <- client$import_table(df1)

R/rdeephaven/inst/tests/testthat/test_client_wrapper.R

@@ -1,7 +1,7 @@
 library(testthat)
 library(rdeephaven)
 
-target <- "localhost:10000"
+target <- get_dh_target()
 
 setup <- function() {
   df1 <- data.frame(
@@ -31,7 +31,6 @@ setup <- function() {
 
 test_that("client dhConnection works in the simple case of anonymous authentication", {
 
-  # TODO: assumes server is actually running on localhost:10000, this is probably bad for CI
   expect_no_error(client <- Client$new(target = target))
 
 })

R/rdeephaven/inst/tests/testthat/test_table_handle_wrapper.R

@@ -23,7 +23,7 @@ setup <- function() {
   )
 
   # set up client
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = get_dh_target())
 
   # move dataframes to server and get TableHandles for testing
   th1 <- client$import_table(df1)

R/rdeephaven/inst/tests/testthat/test_table_ops.R

@@ -38,7 +38,7 @@ setup <- function() {
   )
 
   # set up client
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = get_dh_target())
 
   # move dataframes to server and get TableHandles for testing
   th1 <- client$import_table(df1)

R/rdeephaven/inst/tests/testthat/test_update_by.R

@@ -48,7 +48,7 @@ setup <- function() {
   )
 
   # set up client
-  client <- Client$new(target = "localhost:10000")
+  client <- Client$new(target = get_dh_target())
 
   # move dataframes to server and get TableHandles for testing
   th1 <- client$import_table(df1)

cpp-client/.gitignore

@@ -1,3 +1,4 @@
 *~
 cmake-build-debug
 cmake-build-release
+cmake-build-relwithdebinfo

cpp-client/build.gradle

@@ -7,7 +7,10 @@ plugins {
     id 'license'
 }
 
-evaluationDependsOn Docker.registryProject('cpp-client-base')
+// We use the r-client-base image instead of the cpp-client-base,
+// so that the R client test task in R/build.gradle can in turn use the
+// image we will generate here as a base.
+evaluationDependsOn Docker.registryProject('r-client-base')
 
 configurations {
     cpp {}
@@ -57,10 +60,11 @@ deephavenDocker {
     networkName.set "cpp-test-network-${randomSuffix}"
 }
 
-def testCppClient = Docker.registerDockerTask(project, 'testCppClient') {
+def prefix = '/opt/deephaven'
+
+def buildCppClientImage = Docker.registerDockerTask(project, 'cppClient') {
     // Only tested on x86-64, and we only build dependencies for x86-64
     platform = 'linux/amd64'
-
     copyIn {
         from(layout.projectDirectory) {
             include 'cpp-tests-to-junit.sh'
@@ -71,12 +75,10 @@ def testCppClient = Docker.registerDockerTask(project, 'testCppClient') {
             include 'deephaven/tests/**'
         }
     }
-    copyOut {
-        into layout.buildDirectory.dir('test-results')
-    }
-    def prefix = '/opt/deephaven'
+
     dockerfile {
-        from('deephaven/cpp-client-base:local-build')
+        // See comment at the beginning of this file for why we use this base image.
+        from('deephaven/r-client-base:local-build')
         //
         // Build and install client.
         //
@@ -92,7 +94,7 @@ def testCppClient = Docker.registerDockerTask(project, 'testCppClient') {
         copyFile('deephaven/examples/', "${prefix}/src/deephaven/examples/")
         copyFile('deephaven/tests/', "${prefix}/src/deephaven/tests/")
         copyFile('cpp-tests-to-junit.sh', "${prefix}/bin/dhcpp")
-        runCommand("PREFIX=\"" + prefix + "\"; \\\n" +
+        runCommand("PREFIX=${prefix}; " +
                    '''set -eux; \\
                       rm -fr ${PREFIX}/src/deephaven/build; \\
                       mkdir -p ${PREFIX}/src/deephaven/build; \\
@@ -108,19 +110,35 @@ def testCppClient = Docker.registerDockerTask(project, 'testCppClient') {
                       cd ..; \\
                       rm -fr ${PREFIX}/src/deephaven/build
                    ''')
+        // Note environment variables defined here are inherited by other images
+        // using this image as a base.
+        environmentVariable 'DH_PREFIX', prefix
+        environmentVariable 'LD_LIBRARY_PATH', "${prefix}/lib"
+    }
+    parentContainers = [ Docker.registryTask(project, 'r-client-base') ]
+}
+
+def testCppClient = Docker.registerDockerTask(project, 'testCppClient') {
+    // Only tested on x86-64, and we only build dependencies for x86-64
+    platform = 'linux/amd64'
+    copyIn {}
+    copyOut {
+        into layout.buildDirectory.dir('test-results')
+    }
+    dockerfile {
+        from('deephaven/cpp-client:local-build')
         //
         // Setup for test run.
         //
         environmentVariable 'DH_HOST', deephavenDocker.containerName.get()
         environmentVariable 'DH_PORT', '10000'
-        environmentVariable 'DH_PREFIX', prefix
-        environmentVariable 'LD_LIBRARY_PATH', "${prefix}/lib"
     }
     containerDependencies.dependsOn = [deephavenDocker.healthyTask]
     containerDependencies.finalizedBy = deephavenDocker.endTask
     network = deephavenDocker.networkName.get()
-    parentContainers = [ Docker.registryTask(project, 'cpp-client-base') ]
+    parentContainers = [ project.tasks.getByName('cppClient') ]
     entrypoint = ["${prefix}/bin/dhcpp/cpp-tests-to-junit.sh", '/out/cpp-test.xml', '/out/cpp-test.log']
 }
+
 deephavenDocker.shouldLogIfTaskFails testCppClient
 tasks.check.dependsOn(testCppClient)

cpp-client/cpp-tests-to-junit.sh

@@ -11,7 +11,7 @@ if [ "$#" -ne 2 ]; then
 fi
 
 if [ -z "${DH_PREFIX}" ]; then
-    echo "$0: Environment variable DHCPP_PREFIX is not set, aborting." 1>&2
+    echo "$0: Environment variable DH_PREFIX is not set, aborting." 1>&2
     exit 1
 fi
 

docker/registry/r-client-base/gradle.properties

@@ -0,0 +1,4 @@
+io.deephaven.project.ProjectType=DOCKER_REGISTRY
+deephaven.registry.imageName=ghcr.io/deephaven/r-client-base:latest
+deephaven.registry.imageId=ghcr.io/deephaven/r-client-base@sha256:29cb2969a746cf1f8556b469508a03796297afcfc6fa502622df372ae64d0039
+deephaven.registry.platform=linux/amd64