ropensci-review-tools / autotest

Automatic testing of R packages
https://docs.ropensci.org/autotest
54 stars 5 forks source link

Function accepting a single arbitrary string triggers an issue #65

Closed helske closed 2 years ago

helske commented 2 years ago

I have a few functions in bssm where one argument can be used to define arbitrary names for a vector. This seems to throw a note in autotest:

autotest_package(package = "bssm", functions = "ssm_mng", test = TRUE)

-- autotesting bssm --

v [1 / 1]: ssm_mng
# A tibble: 1 x 9
  type       test_name          fn_name parameter   parameter_type   operation                            content           test  yaml_hash      
  <chr>      <chr>              <chr>   <chr>       <chr>            <chr>                                <chr>             <lgl> <chr>          
1 diagnostic random_char_string ssm_mng state_names single character random character string as parameter does not match a~ TRUE  b9e6da23bdae30~

I'm not sure what would be to correct solution here?

mpadge commented 2 years ago

I'm a bit unsure about this one too. It happens because current code just checks for match.arg, and issues a diagnostic if that's not used. That's obviously inadequate, and maybe the best approach might be a more sophisticated grep of doc entries, like for #66? We'd just need to establish a condition under which match.arg tests would not be run, maybe something like whenever param descriptions include the terms "name" or "label"? What do you think? An alternative would be to try to diagnose when match.arg should be checked, perhaps via a grep for things like "class" or "type", but that seems more dangerous and restrictive.

Proposal

chk_match_arg <- !grepl("name|label|text|string", rd_text,, ignore.case = TRUE)
helske commented 2 years ago

Yes, this is tricky. I think the word "arbitrary" could be checked as that would be a pretty clear indication that everything should be accepted. I can also envision cases where the argument is used for subsetting vector, data frame etc (the argument should be a column name present in some other input data). In these cases, we should see an error if we modify the argument value randomly, but the function could use other mechanisms than match.arg for checking the validity of the input.