From 0b49cb2f524555f2fca3ef1c3b1ad1f7d3134cb3 Mon Sep 17 00:00:00 2001
From: Nick-Eagles <nickeagles77@gmail.com>
Date: Thu, 18 Jul 2024 14:58:52 -0400
Subject: [PATCH] Fix unit tests after interactive testing; fix a bug with
 trailing commas in overlap_key in response to unit-test failure

---
 R/add_overlap_info.R                   |  5 ++++-
 tests/testthat/test-add_array_coords.R |  8 +++----
 tests/testthat/test-add_overlap_info.R | 29 ++++++++++++++++++++------
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/R/add_overlap_info.R b/R/add_overlap_info.R
index 4576392..bf87dd7 100644
--- a/R/add_overlap_info.R
+++ b/R/add_overlap_info.R
@@ -76,10 +76,13 @@ add_overlap_info <- function(spe, metric_name) {
             sub(col_data$key[i], "", col_data$overlap_key[i], fixed = TRUE)
         }
     )
+
+    #   Remove double commas (only possible in the middle) or leading/trailing
+    #   commas
     col_data$overlap_key <- sub(
         ",,",
         ",",
-        sub("^,", "", col_data$overlap_key)
+        sub("^,|,$", "", col_data$overlap_key)
     )
 
     ############################################################################
diff --git a/tests/testthat/test-add_array_coords.R b/tests/testthat/test-add_array_coords.R
index ce20437..1782fda 100644
--- a/tests/testthat/test-add_array_coords.R
+++ b/tests/testthat/test-add_array_coords.R
@@ -47,10 +47,10 @@ test_that(
             spe, sample_info, spe_input_dir, overwrite = FALSE
         )
 
-        #   12 columns should've been added, matching the specific naming
+        #   10 columns should've been added, matching the specific naming
         #   pattern
         expect_equal(
-            length(grep(added_cols_regex, colnames(colData(spe_new)))), 12
+            length(grep(added_cols_regex, colnames(colData(spe_new)))), 10
         )
 
         #   Array and spatial coords shouldn't be overwritten
@@ -62,10 +62,10 @@ test_that(
             spe, sample_info, spe_input_dir, overwrite = TRUE
         )
 
-        #   12 columns should've been added, matching the specific naming
+        #   10 columns should've been added, matching the specific naming
         #   pattern
         expect_equal(
-            length(grep(added_cols_regex, colnames(colData(spe_new)))), 12
+            length(grep(added_cols_regex, colnames(colData(spe_new)))), 10
         )
 
         #   spatialCoords should be updated with transformed coordinates
diff --git a/tests/testthat/test-add_overlap_info.R b/tests/testthat/test-add_overlap_info.R
index 58990c4..73d4380 100644
--- a/tests/testthat/test-add_overlap_info.R
+++ b/tests/testthat/test-add_overlap_info.R
@@ -13,23 +13,40 @@ test_that(
         #   V13B23-283_A1 should have excluded spots (lowest in my_metric) while
         #   V13B23-283_D1 should have none
         expect_equal(
-            any(spe$exclude_overlapping[spe$capture_area == "V13B23-283_A1"]),
+            any(spe_new$exclude_overlapping[spe_new$capture_area == "V13B23-283_A1"]),
             TRUE
         )
         expect_equal(
-            any(spe$exclude_overlapping[spe$capture_area == "V13B23-283_D1"]),
+            any(spe_new$exclude_overlapping[spe_new$capture_area == "V13B23-283_D1"]),
             FALSE
         )
 
-        overlap_keys = colData(spe) |>
+        #   'overlap_key' should consist of a comma-separated list of valid keys
+        #   (or the empty string)
+        one_key_regex = '[ACTG]+-1_V13B23-283_[ACD]1'
+        expect_equal(
+            all(
+                grepl(
+                    gsub('X', one_key_regex, '^(|X|X(,X)*)$'),
+                    spe_new$overlap_key
+                )
+            ),
+            TRUE
+        )
+
+        overlap_keys = colData(spe_new) |>
+            as_tibble() |>
             filter(capture_area == "V13B23-283_A1", overlap_key != "") |>
             pull(overlap_key) |>
             paste(collapse = ",") |>
-            str_split(',')
+            strsplit(',')
         
         #   All keys overlapping V13B23-283_A1 should exist in the object and
-        #   belong to capture area V13B23-283_D1
+        #   belong to capture area V13B23-283_D1 (or rarely V13B23-283_A1,
+        #   because of precision issues in pixel coordinates)
         expect_equal(all(overlap_keys[[1]] %in% spe$key), TRUE)
-        expect_equal(all(str_split_i(overlap_keys[[1]], '_', 2) == "D1"), TRUE)
+        expect_equal(
+            all(grepl('[ACTG]+-1_V13B23-283_[AD]1', overlap_keys[[1]])), TRUE
+        )
     }
 )