docs: improve on error message

insightsengineering · Oct 19, 2023 · a76a140 · a76a140
1 parent 2f0ffe4
commit a76a140
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 49 deletions.
diff --git a/R/JoinKeys.R b/R/JoinKeys.R
@@ -302,7 +302,7 @@ JoinKeys <- R6::R6Class( # nolint
         }
 
         if (xor(length(join_key_1$keys) == 0, length(join_key_2$keys) == 0) ||
-          !identical(sort(join_key_1$keys), sort(setNames(names(join_key_2$keys), join_key_2$keys)))) {
+              !identical(sort(join_key_1$keys), sort(setNames(names(join_key_2$keys), join_key_2$keys)))) {
           error_message(join_key_1$dataset_1, join_key_1$dataset_2)
         }
       }
@@ -490,8 +490,8 @@ join_key <- function(dataset_1, dataset_2 = NULL, keys) {
 #'
 primary_key <- function(dataset_1, keys) {
   if (checkmate::test_character(keys) &&
-    !checkmate::test_names(names(keys), type = "unnamed")) {
-    stop("Primary keys must match exactly: keys = c('A' = 'B') are not allowed")
+        !checkmate::test_names(names(keys), type = "unnamed")) {
+    stop("Primary keys parameter must be a unamed character vector: keys = c('A' = 'A') are not allowed")
   }
   join_key(dataset_1 = dataset_1, dataset_2 = dataset_1, keys = keys)
 }
diff --git a/tests/testthat/test-JoinKeys.R b/tests/testthat/test-JoinKeys.R
@@ -271,33 +271,35 @@ testthat::test_that("JoinKeys$split method returns a named list of JoinKeys obje
   testthat::expect_equal(names(res$Y$get()), c("Y", "Z"))
 })
 
-testthat::test_that("JoinKeys$split method returns an updated list after
-  the state of the object is modified by JoinKeys$mutate()", {
-  x <- JoinKeys$new()
-  x$set(
-    list(
-      join_key("A", "B", c("a" = "b")),
-      join_key("A", "C", c("a" = "c", "aa" = "cc")),
-      join_key("Z", "Y", c("z" = "y"))
+testthat::test_that(
+  "JoinKeys$split method returns an updated list after the state of the object is modified by JoinKeys$mutate()",
+  {
+    x <- JoinKeys$new()
+    x$set(
+      list(
+        join_key("A", "B", c("a" = "b")),
+        join_key("A", "C", c("a" = "c", "aa" = "cc")),
+        join_key("Z", "Y", c("z" = "y"))
+      )
     )
-  )
-  res <- x$split()
+    res <- x$split()
 
-  x$mutate("A", "B", c("a" = "b", "aa" = "bb"))
-  res2 <- x$split()
+    x$mutate("A", "B", c("a" = "b", "aa" = "bb"))
+    res2 <- x$split()
 
-  testthat::expect_false(identical(res, res2))
-  testthat::expect_identical(res2$A$get()$A$B, c("a" = "b", "aa" = "bb"))
+    testthat::expect_false(identical(res, res2))
+    testthat::expect_identical(res2$A$get()$A$B, c("a" = "b", "aa" = "bb"))
 
-  # adding new datasets
-  x$mutate("D", "G", c("d" = "g"))
-  res3 <- x$split()
-  testthat::expect_false(identical(res, res3))
-  testthat::expect_false(identical(res2, res3))
-  testthat::expect_identical(res3$D$get()$D$G, c("d" = "g"))
-  testthat::expect_identical(res3$D$get()$G$D, c("g" = "d"))
-  testthat::expect_identical(names(res3$D$get()), c("D", "G"))
-})
+    # adding new datasets
+    x$mutate("D", "G", c("d" = "g"))
+    res3 <- x$split()
+    testthat::expect_false(identical(res, res3))
+    testthat::expect_false(identical(res2, res3))
+    testthat::expect_identical(res3$D$get()$D$G, c("d" = "g"))
+    testthat::expect_identical(res3$D$get()$G$D, c("g" = "d"))
+    testthat::expect_identical(names(res3$D$get()), c("D", "G"))
+  }
+)
 
 testthat::test_that("JoinKeys$split method does not modify self", {
   x <- JoinKeys$new()
@@ -361,35 +363,37 @@ testthat::test_that("JoinKeys$merge can handle edge case: argument is a list of
   testthat::expect_identical(previous_output, y$get())
 })
 
-testthat::test_that("JoinKeys$merge throws error when improper argument is
-  passed in without modifying the caller", {
-  y <- JoinKeys$new()
-  y$set(
-    list(
-      join_key("A", "B", c("a" = "b")),
-      join_key("A", "C", c("a" = "c", "aa" = "cc")),
-      join_key("Z", "Y", c("z" = "y"))
+testthat::test_that(
+  "JoinKeys$merge throws error when improper argument is passed in without modifying the caller",
+  {
+    y <- JoinKeys$new()
+    y$set(
+      list(
+        join_key("A", "B", c("a" = "b")),
+        join_key("A", "C", c("a" = "c", "aa" = "cc")),
+        join_key("Z", "Y", c("z" = "y"))
+      )
     )
-  )
-  previous_output <- y$get()
-  testthat::expect_error(y$merge())
-  testthat::expect_identical(previous_output, y$get())
+    previous_output <- y$get()
+    testthat::expect_error(y$merge())
+    testthat::expect_identical(previous_output, y$get())
 
-  testthat::expect_error(y$merge(1))
-  testthat::expect_identical(previous_output, y$get())
+    testthat::expect_error(y$merge(1))
+    testthat::expect_identical(previous_output, y$get())
 
-  testthat::expect_error(y$merge("A"))
-  testthat::expect_identical(previous_output, y$get())
+    testthat::expect_error(y$merge("A"))
+    testthat::expect_identical(previous_output, y$get())
 
-  testthat::expect_error(y$merge(list()))
-  testthat::expect_identical(previous_output, y$get())
+    testthat::expect_error(y$merge(list()))
+    testthat::expect_identical(previous_output, y$get())
 
-  testthat::expect_error(y$merge(list(1)))
-  testthat::expect_identical(previous_output, y$get())
+    testthat::expect_error(y$merge(list(1)))
+    testthat::expect_identical(previous_output, y$get())
 
-  testthat::expect_error(y$merge(list("A")))
-  testthat::expect_identical(previous_output, y$get())
-})
+    testthat::expect_error(y$merge(list("A")))
+    testthat::expect_identical(previous_output, y$get())
+  }
+)
 
 testthat::test_that("JoinKeys$merge does nothing when argument is a JoinKeys object with identical data", {
   x <- JoinKeys$new()