430 use expect_no_error in tests instead of expect_error(, NA) (#173)

Part of insightsengineering/coredev-tasks#430

---------

Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: André Veríssimo <211358+averissimo@users.noreply.github.com>

Author: 3 people

DESCRIPTION

@@ -58,4 +58,4 @@ Encoding: UTF-8
 Language: en-US
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.3
+RoxygenNote: 7.3.0

tests/testthat/test-all_choices.R

@@ -1,3 +1,3 @@
 testthat::test_that("all_choices constructor does not throw", {
-  testthat::expect_error(all_choices(), NA)
+  testthat::expect_no_error(all_choices())
 })

tests/testthat/test-data_extract_spec.R

@@ -1,5 +1,5 @@
 testthat::test_that("data_extract_spec throws when select is not select_spec or NULL", {
-  expect_error(data_extract_spec("toyDataset", select = c("A", "B")))
+  testthat::expect_error(data_extract_spec("toyDataset", select = c("A", "B")))
 })
 
 testthat::test_that("data_extract_spec works with valid input", {
@@ -301,7 +301,7 @@ testthat::test_that("delayed version of data_extract_spec", {
 })
 
 testthat::test_that("data_extract_spec allows both select and filter parameters to be NULL", {
-  testthat::expect_error(des <- data_extract_spec("ADSL"), NA)
+  testthat::expect_no_error(des <- data_extract_spec("ADSL"))
 })
 
 testthat::test_that("data_extract_spec returns filter_spec with multiple set to TRUE", {

tests/testthat/test-filter_spec.R

@@ -176,7 +176,7 @@ testthat::test_that("delayed filter_spec", {
     multiple = FALSE
   )
 
-  expect_equal(names(expected_spec), names(delayed))
+  testthat::expect_equal(names(expected_spec), names(delayed))
 
   data_list <- list(ADSL = reactive(ADSL))
   key_list <- list(ADSL = c("STUDYID", "USUBJID"))
@@ -191,7 +191,7 @@ testthat::test_that("filter_spec with choices_selected where all selected in cho
     choices = stats::setNames(LETTERS[1:5], paste("Letter", LETTERS[1:5])),
     selected = c("A", "B")
   )
-  testthat::expect_error(filter_spec(vars = valid_cs), regexp = NA)
+  testthat::expect_no_error(filter_spec(vars = valid_cs))
 })
 
 
@@ -224,7 +224,7 @@ testthat::test_that("filter_spec_internal contains dataname", {
     vars_choices = variable_choices(ADSL)
   )
 
-  expect_null(x_filter$dataname)
+  testthat::expect_null(x_filter$dataname)
 
   x <- data_extract_spec(
     dataname = "ADSL",
@@ -477,7 +477,7 @@ testthat::test_that(
       choices = stats::setNames(LETTERS[1:5], paste("Letter", LETTERS[1:5])),
       selected = c("A", "B")
     )
-    testthat::expect_error(filter_spec(vars = valid_cs), regexp = NA)
+    testthat::expect_no_error(filter_spec(vars = valid_cs))
   }
 )
 

tests/testthat/test-format_data_extract.R

@@ -3,7 +3,7 @@ required_names <- c("select", "filters", "dataname")
 testthat::test_that("format_data_extract is a function that accepts a list", {
   data_extract_fake <- as.list(stats::setNames(nm = required_names))
   data_extract_fake$filters <- list()
-  testthat::expect_error(format_data_extract(data_extract_fake), regexp = NA)
+  testthat::expect_no_error(format_data_extract(data_extract_fake))
 })
 
 testthat::test_that("format_data_extract asserts its argument has required names", {
@@ -46,7 +46,7 @@ testthat::test_that("format_data_extract integrates with data_extract_srv", {
     data_extract_srv,
     args = list(data_extract_spec = simple_des, datasets = sample_data),
     expr = {
-      testthat::expect_error(format_data_extract(session$returned()), regexp = NA)
+      testthat::expect_no_error(format_data_extract(session$returned()))
     }
   )
 })

tests/testthat/test-merge_expression_module.R

@@ -37,13 +37,12 @@ testthat::test_that(
   code = {
     shiny::withReactiveDomain(
       domain = shiny::MockShinySession$new(),
-      expr = testthat::expect_error(
+      expr = testthat::expect_no_error(
         merge_expression_module(
           data_extract = list(adsl_var = adsl_extract, adlb_var = adlb_extract),
           datasets = data_list,
           join_keys = join_keys
-        ),
-        NA
+        )
       )
     )
   }

tests/testthat/test-merge_expression_srv.R

@@ -173,16 +173,15 @@ testthat::test_that("merge_expression_srv throws error if selector_list is not n
 
 testthat::test_that("merge_expression_srv accepts reactive and character merge_function", {
   m_fun <- reactive("dplyr::left_join")
-  testthat::expect_error(
+  testthat::expect_no_error(
     shiny::testServer(
       merge_expression_srv,
       args = list(selector_list = selector_list, datasets = data_list, join_keys = join_keys, merge_function = m_fun),
       expr = session$returned()
-    ),
-    NA
+    )
   )
 
-  testthat::expect_error(
+  testthat::expect_no_error(
     shiny::testServer(
       merge_expression_srv,
       args = list(
@@ -192,8 +191,7 @@ testthat::test_that("merge_expression_srv accepts reactive and character merge_f
         merge_function = "dplyr::left_join"
       ),
       expr = session$returned()
-    ),
-    NA
+    )
   )
 })
 
@@ -243,21 +241,19 @@ testthat::test_that("merge_expression_srv throws error if join_keys is not a joi
 })
 
 testthat::test_that("merge_expression_srv accepts a list of (reactive) data.frames for datasets argument", {
-  testthat::expect_error(
+  testthat::expect_no_error(
     shiny::testServer(
       merge_expression_srv,
       args = list(selector_list = selector_list, datasets = data_list, join_keys = join_keys),
       expr = NULL
-    ),
-    NA
+    )
   )
 
-  testthat::expect_error(
+  testthat::expect_no_error(
     shiny::testServer(
       merge_expression_srv,
       args = list(selector_list = selector_list, datasets = data_list_nr, join_keys = join_keys),
       expr = NULL
-    ),
-    NA
+    )
   )
 })

tests/testthat/test-select_spec.R

@@ -79,7 +79,7 @@ testthat::test_that("Multiple choices", {
 
   testthat::expect_true(c1$multiple)
   testthat::expect_false(c1$fixed)
-  expect_null(c1$always_selected)
+  testthat::expect_null(c1$always_selected)
   testthat::expect_false(c1$ordered)
   testthat::expect_identical(c1$label, "Select")
 })

tests/testthat/test-value_choices.R

@@ -22,14 +22,14 @@ testthat::test_that("Will output warnings when value_choices applied on datasets
   testthat::expect_warning(value_choices(data, var_choices = c("A", "F")))
   testthat::expect_error(value_choices(data, var_choices = "K"))
   testthat::expect_error(value_choices(data, var_choices = "F", var_label = "K"))
-  testthat::expect_warning(value_choices(data, var_choices = c("J")), NA)
-  testthat::expect_warning(value_choices(data, var_choices = c("B")), NA)
+  testthat::expect_no_warning(value_choices(data, var_choices = c("J")))
+  testthat::expect_no_warning(value_choices(data, var_choices = c("B")))
 })
 
 testthat::test_that("delayed version of value_choices", {
   # hard-coded subset
   obj <- value_choices("ADSL", var_choices = "ARMCD", var_label = "ARM", subset = c("ARM A", "ARM B"))
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(
@@ -131,7 +131,7 @@ testthat::test_that("delayed version of value_choices", {
 testthat::test_that("delayed version of value_choices - resolve_delayed", {
   # hard-coded subset
   obj <- value_choices("ADSL", var_choices = "ARMCD", var_label = "ARM", subset = c("ARM A", "ARM B"))
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(

tests/testthat/test-variable_choices.R

@@ -8,25 +8,25 @@ test_that("Can create variable_choices with datasets with no or missing labels",
 
   # no labels given
   choice_1 <- variable_choices(example_data, fill = TRUE)
-  expect_equal(names(choice_1), c("USUBJID: USUBJID", "STUDYID: STUDYID"))
+  testthat::expect_equal(names(choice_1), c("USUBJID: USUBJID", "STUDYID: STUDYID"))
 
   # one missing label
   missing_one_label_data <- example_data
   teal.data::col_labels(missing_one_label_data) <- c(as.character(NA), "Label")
   choice_2 <- variable_choices(missing_one_label_data, fill = FALSE)
-  expect_equal(names(choice_2), c("USUBJID: Label Missing", "STUDYID: Label"))
+  testthat::expect_equal(names(choice_2), c("USUBJID: Label Missing", "STUDYID: Label"))
 
   # all missing label
   missing_two_label_data <- example_data
   teal.data::col_labels(missing_two_label_data) <- c(as.character(NA), as.character(NA))
   choice_2 <- variable_choices(missing_two_label_data, fill = FALSE)
-  expect_equal(names(choice_2), c("USUBJID: Label Missing", "STUDYID: Label Missing"))
+  testthat::expect_equal(names(choice_2), c("USUBJID: Label Missing", "STUDYID: Label Missing"))
 })
 
 test_that("delayed version of variable_choices", {
   # hard-coded subset
   obj <- variable_choices("ADSL", subset = c("SEX", "ARMCD", "COUNTRY"))
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(data = "ADSL", subset = c("SEX", "ARMCD", "COUNTRY"), key = NULL),
@@ -38,14 +38,14 @@ test_that("delayed version of variable_choices", {
   key_list <- list(ADSL = c("STUDYID", "USUBJID"), ADTTE = c("STUDYID", "USUBJID", "PARAMCD"))
 
   res_obj <- isolate(resolve(obj, datasets = data_list, keys = key_list))
-  expect_equal(
+  testthat::expect_equal(
     res_obj,
     variable_choices(ADSL, subset = c("SEX", "ARMCD", "COUNTRY"))
   )
 
   # functional subset
   obj <- variable_choices("ADSL", subset = function(data) colnames(data)[1:2])
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(data = "ADSL", subset = function(data) colnames(data)[1:2], key = NULL),
@@ -54,14 +54,14 @@ test_that("delayed version of variable_choices", {
   )
 
   res_obj <- isolate(resolve(obj, datasets = data_list, keys = key_list))
-  expect_equal(
+  testthat::expect_equal(
     res_obj,
     variable_choices(ADSL, subset = colnames(ADSL)[1:2], key = c("STUDYID", "USUBJID"))
   )
 
   # non-null key value
   obj <- variable_choices("ADSL", key = c("USUBJID", "STUDYID"))
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(data = "ADSL", subset = NULL, key = c("USUBJID", "STUDYID")),
@@ -70,7 +70,7 @@ test_that("delayed version of variable_choices", {
   )
 
   res_obj <- isolate(resolve(obj, datasets = data_list, keys = key_list))
-  expect_equal(
+  testthat::expect_equal(
     res_obj,
     variable_choices(ADSL, key = c("USUBJID", "STUDYID"))
   )
@@ -80,7 +80,7 @@ test_that("delayed version of variable_choices", {
 test_that("delayed version of variable_choices - resolve_delayed", {
   # hard-coded subset
   obj <- variable_choices("ADSL", subset = c("SEX", "ARMCD", "COUNTRY"))
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(data = "ADSL", subset = c("SEX", "ARMCD", "COUNTRY"), key = NULL),
@@ -89,15 +89,15 @@ test_that("delayed version of variable_choices - resolve_delayed", {
   )
 
   res_obj <- isolate(resolve_delayed(obj, datasets = data_list, keys = primary_keys_list))
-  expect_equal(
+  testthat::expect_equal(
     res_obj,
     variable_choices(ADSL, subset = c("SEX", "ARMCD", "COUNTRY"))
   )
 
 
   # functional subset
   obj <- variable_choices("ADSL", subset = function(data) colnames(data)[1:2])
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(data = "ADSL", subset = function(data) colnames(data)[1:2], key = NULL),
@@ -106,14 +106,14 @@ test_that("delayed version of variable_choices - resolve_delayed", {
   )
 
   res_obj <- isolate(resolve_delayed(obj, datasets = data_list, keys = primary_keys_list))
-  expect_equal(
+  testthat::expect_equal(
     res_obj,
     variable_choices(ADSL, subset = colnames(ADSL)[1:2], key = c("STUDYID", "USUBJID"))
   )
 
   # non-null key value
   obj <- variable_choices("ADSL", key = c("USUBJID", "STUDYID"))
-  expect_equal(
+  testthat::expect_equal(
     obj,
     structure(
       list(data = "ADSL", subset = NULL, key = c("USUBJID", "STUDYID")),
@@ -122,7 +122,7 @@ test_that("delayed version of variable_choices - resolve_delayed", {
   )
 
   res_obj <- isolate(resolve_delayed(obj, datasets = data_list, keys = primary_keys_list))
-  expect_equal(
+  testthat::expect_equal(
     res_obj,
     variable_choices(ADSL, key = c("USUBJID", "STUDYID"))
   )