From ec66be6835375905e979d71fcff5d6aa53bb61c5 Mon Sep 17 00:00:00 2001
From: Siddhartha Bagaria <sbagaria@grailbio.com>
Date: Thu, 5 Mar 2020 21:24:08 -0800
Subject: [PATCH] rules_r: fix binary_image and write tests

binary_image rule has been broken after the flag
incompatible_depset_is_not_iterable was flipped.

This change fixes the implementation and also introduces some tests for
images. In writing these tests, another breakage has been discovered
because of the flag flip of experimental_remap_main_repo. This bug will
be fixed at a later date.
---
 R/container/binary.bzl        |  2 +-
 tests/container/BUILD         |  4 ++--
 tests/container/image_test.sh | 44 ++++++++++++++++++++++++++++++++---
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/R/container/binary.bzl b/R/container/binary.bzl
index 4493d70..e28d05f 100644
--- a/R/container/binary.bzl
+++ b/R/container/binary.bzl
@@ -35,7 +35,7 @@ def _binary_layer_impl(ctx):
         directory = "/",
         file_map = {
             _layer_file_path(ctx, f): f
-            for f in ctx.attr.binary[OutputGroupInfo][ctx.attr.layer_type]
+            for f in ctx.attr.binary[OutputGroupInfo][ctx.attr.layer_type].to_list()
         },
     )
 
diff --git a/tests/container/BUILD b/tests/container/BUILD
index 0fa9641..7079520 100644
--- a/tests/container/BUILD
+++ b/tests/container/BUILD
@@ -42,13 +42,13 @@ sh_test(
     size = "small",
     srcs = ["image_test.sh"],
     data = [
+        ":binary_image",
+        ":binary_image_explicit_layers",
         ":library_archive",
         ":library_image",
     ],
 )
 
-# TODO: Introduce tests for binary_image after moving to rules_docker.
-
 r_binary_image(
     name = "binary_image",
     base = "@r_base//image",
diff --git a/tests/container/image_test.sh b/tests/container/image_test.sh
index 6515edc..6a35814 100755
--- a/tests/container/image_test.sh
+++ b/tests/container/image_test.sh
@@ -33,11 +33,13 @@ check() {
     echo "${TAR}" not found.
     exit 1
   fi
-  echo "Looking for file ${FILE_TO_CHECK} in ${TAR}:"
+  printf "Looking for file %s in %s: " "${FILE_TO_CHECK}" "${TAR}"
   if tar -tf "${TAR}" "${FILE_TO_CHECK}" >/dev/null 2>/dev/null; then
-    $EXPECT || (echo "Found but not expecting!" && exit 1)
+    ($EXPECT && echo "Found and expecting") || (echo "Found but not expecting!" && exit 1)
   elif $EXPECT; then
     echo "Not found!" && exit 1
+  else
+    echo "Not found and not expecting"
   fi
 }
 
@@ -49,4 +51,40 @@ check "library_image_internal-layer.tar" "./grail/r-libs/bitops/DESCRIPTION" "fa
 check "library_archive.tar.gz" "./grail/r-libs/exampleC/DESCRIPTION" "false"
 check "library_archive.tar.gz" "./grail/r-libs/bitops/DESCRIPTION" "true"
 
-echo "Found!"
+# TODO: Making an image by repo is not working as intended because all files
+# are also present in the topmost layer.
+
+# Check binary script and R library packages.
+check "binary_image-layer.tar" "/app/binary/binary" "true"
+check "binary_image-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/binary/binary.R" "true"
+check "binary_image-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/exampleA/lib/exampleA/DESCRIPTION" "true"
+check "binary_image-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/exampleB/lib/exampleB/DESCRIPTION" "true"
+check "binary_image-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/exampleC/lib/exampleC/DESCRIPTION" "true"
+check "binary_image-layer.tar" "./app/binary/binary.runfiles/R_bitops/lib/bitops/DESCRIPTION" "true"
+check "binary_image-layer.tar" "./app/binary/binary.runfiles/R_R6/lib/R6/DESCRIPTION" "true"
+
+# TODO: Making an image by explicit layers is currently broken because the
+# package files are not symlinked correctly because of change in behavior re
+# remap_main_repo.
+
+# Check that explicitly layered packages are not in the top layer, but in one layer below.
+check "binary_image_explicit_layers-layer.tar" "/app/binary/binary" "true"
+check "binary_image_explicit_layers-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/binary/binary.R" "true"
+check "binary_image_explicit_layers-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/exampleA/lib/exampleA/DESCRIPTION" "true"
+check "binary_image_explicit_layers-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/exampleB/lib/exampleB/DESCRIPTION" "true"
+check "binary_image_explicit_layers-layer.tar" "./app/binary/binary.runfiles/com_grail_rules_r_tests/exampleC/lib/exampleC/DESCRIPTION" "true"
+check "binary_image_explicit_layers-layer.tar" "./app/binary/binary.runfiles/R_bitops/lib/bitops/DESCRIPTION" "false"
+check "binary_image_explicit_layers-layer.tar" "./app/binary/binary.runfiles/R_R6/lib/R6/DESCRIPTION" "false"
+#check "binary_image_explicit_layers.0-layer.tar" "./app/binary/binary.runfiles/R_bitops/lib/bitops/DESCRIPTION" "true"
+check "binary_image_explicit_layers.0-layer.tar" "./app/binary/binary.runfiles/R_R6/lib/R6/DESCRIPTION" "false"
+check "binary_image_explicit_layers.1-layer.tar" "./app/binary/binary.runfiles/R_bitops/lib/bitops/DESCRIPTION" "false"
+#check "binary_image_explicit_layers.1-layer.tar" "./app/binary/binary.runfiles/R_R6/lib/R6/DESCRIPTION" "true"
+
+# TODO: fix image by explicit layers (see also TODO above).
+
+if docker --version; then
+  binary_image.executable
+  # binary_image_explicit_layers.executable
+fi
+
+echo "SUCCESS!"