assign_test_ids misfire if all tests have IDs

[kyleam]

cc @barrettk

When I was looking at #39, I hit into a case where calling assign_test_ids triggers spurious replacements. I believe this is when an empty TestIds data frame goes into overwrite_tests, leading to a character(0) value for from and to in replace_test_str.

---

Example on current develop (3c7c65c):

assign_test_ids(test_path = system.file(
  "fake-tests", "new_tests", package = "mrgvalprep"))

diff

```diff diff --git a/inst/fake-tests/new_tests/test-copy-model-from.R b/inst/fake-tests/new_tests/test-copy-model-from.R index c7e766b..affcbaf 100644 --- a/inst/fake-tests/new_tests/test-copy-model-from.R +++ b/inst/fake-tests/new_tests/test-copy-model-from.R @@ -34,7 +34,7 @@ # check the control stream is modified new_mod_str <- ctl_ext(NEW_MOD2) %>% readr::read_file() - new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " ", NEW_DESC, "\n\n\\$INPUT") + new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " []", NEW_DESC, "\n\n\\$INPUT") expect_true(grepl(new_desc_pattern, new_mod_str)) }) @@ -92,7 +92,7 @@ # check the control stream is modified new_mod_str <- readr::read_file(new_ctl_path) - new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " ", NEW_DESC, "\n\n\\$INPUT") + new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " []", NEW_DESC, "\n\n\\$INPUT") expect_true(grepl(new_desc_pattern, new_mod_str)) }) @@ -119,7 +119,7 @@ orig_desc_pattern <- paste0("\\$PROBLEM ", DESC_IN_CTL, "\n\n\\$INPUT") expect_false(grepl(orig_desc_pattern, new_mod_str)) - new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " ", NEW_DESC, "\n\n\\$INPUT") + new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " []", NEW_DESC, "\n\n\\$INPUT") expect_true(grepl(new_desc_pattern, new_mod_str)) }) @@ -148,7 +148,7 @@ orig_desc_pattern <- paste0("\\$PROBLEM ", DESC_IN_CTL, "\n\n\\$INPUT") expect_true(grepl(orig_desc_pattern, new_mod_str)) - new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " ", NEW_DESC, "\n\n\\$INPUT") + new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " []", NEW_DESC, "\n\n\\$INPUT") expect_false(grepl(new_desc_pattern, new_mod_str)) }) }) # closing withr::with_options @@ -159,8 +159,8 @@ on.exit({ cleanup() }) # get integer input and check for related paths - mod1 <- stringr::str_replace_all(YAML_TEST_FILE, "[^\\d]", "") %>% as.numeric() - mod2 <- stringr::str_replace_all(NEW_MOD2, "[^\\d]", "") %>% as.numeric() + mod1 <- stringr::str_replace_all(YAML_TEST_FILE, "[^\\d]", " []") %>% as.numeric() + mod2 <- stringr::str_replace_all(NEW_MOD2, "[^\\d]", " []") %>% as.numeric() new_yaml_path <- yaml_ext(NEW_MOD2) new_ctl_path <- ctl_ext(NEW_MOD2) expect_false(fs::file_exists(new_yaml_path)) @@ -180,7 +180,7 @@ # check the control stream is modified new_mod_str <- readr::read_file(new_ctl_path) - new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " ", NEW_DESC, "\n\n\\$INPUT") + new_desc_pattern <- paste0("\\$PROBLEM ", get_model_id(NEW_MOD2), " []", NEW_DESC, "\n\n\\$INPUT") expect_true(grepl(new_desc_pattern, new_mod_str)) }) }) # closing withr::with_options diff --git a/inst/fake-tests/new_tests/test-workflow-dryrun.R b/inst/fake-tests/new_tests/test-workflow-dryrun.R index f6f0944..0d4c263 100644 --- a/inst/fake-tests/new_tests/test-workflow-dryrun.R +++ b/inst/fake-tests/new_tests/test-workflow-dryrun.R @@ -33,8 +33,8 @@ readr::write_file("created_by: test-workflow-dryrun part 1", bbi_yaml) for (.test_case in .TEST_CASES_WD) { - test_that(paste0("basic workflow is correct using rbabylon.model_directory = '", - MODEL_DIR, "'", .test_case$test_wd, + test_that(paste0("basic workflow is correct using rbabylon.model_directory = ' []", + MODEL_DIR, " []'", .test_case$test_wd, if(isTRUE(.test_case$change_midstream)) " change_midstream"), { withr::with_dir(.test_case$test_wd, { @@ -86,7 +86,7 @@ } for (.test_case in .TEST_CASES_WD) { - test_that(paste(glue::glue("summary call is the same using rbabylon.model_directory = '{MODEL_DIR}'"), .test_case$test_wd, if(isTRUE(.test_case$change_midstream)) "change_midstream"), { + test_that(paste(glue::glue("summary call is the same using rbabylon.model_directory = '{MODEL_DIR}' []"), .test_case$test_wd, if(isTRUE(.test_case$change_midstream)) "change_midstream"), { withr::with_dir(.test_case$test_wd, { # load mod from yaml and dry run through to summary object ```

[barrettk]

Great thanks for reporting. If it indeed only shows up when the dataframe is empty I can add something simple to prevent that case. Will keep an eye out for this as I test pmtables and the other packages

[kyleam]

Fixed by #42.