Testing for method existence

[Puzzled-Face]

The unit testing in crmPack currently focuses on the correct operation of methods and functions that exist. It does not check that all functions that should exist actually do exist. A recent example of this was the need to define some missing default constructors [Issue #659].

Similarly, we currently rely on good review to ensure that all required methods for a new class have been implemented. [For example, dose, prob and fit methods for a new Model/Data combination.] I think it would be helpful to automate such checking.

Consider

#' Tests For Existence of Required Method for ALl Relevant Subclasses
#' 
#' Examines all subclasses of the parent superclass to determine if a method 
#' with the given name and signature exists.  '{cls}' is used as a placeholder
#' for the subclass name in the required signature
#' 
#' @param method_name ('character')\cr the name of the method
#' @param parent_class ('character')\cr the name of the parent_class
#' @param signature ('character')\cr vector defining the default signature of 
#' the method
#' @param exclude ('character')\cr a scalar or vector defining the subclasses for
#' no method is expected to exist and therefore the test should be skipped.  Use
#' only for virtual subclasses 
#' @param exceptions (`character`) a named list defining the class specific signatures
#' for which the default signature should not be used.  The names of the list define
#' the class name, the corresponding value the signature to be used.
#' @returns `TRUE` if all required methods exist, otherwise a character of the form
#' `method <xxx> not found for classes <yyy>, <zzz>`
#' @internal
h_all_required_methods_exist <- function(method_name, parent_class, signature, exclude = c(), exceptions = list()) {
  subclass_names <- names(getClass(parent_class)@subclasses)
  subclass_names <- setdiff(subclass_names, as.vector(exclude, "character"))
  methods_exist <- unlist(
    lapply(
      subclass_names,
      function(cls) {
        this_signature <- if(cls %in% names(exceptions)) {
          this_signature <- exceptions[[cls]]
        } else {
          this_signature <- signature
        } 
        this_signature <- ifelse(this_signature == "{cls}", cls, this_signature)
        hasMethod(method_name, this_signature)
      }
    )
  )
  names(methods_exist) <- subclass_names
  if (all(methods_exist)) {
    return(TRUE)
  } 
  missing_classes <- which(!methods_exist)
  paste0(
    "method \"", 
    method_name, 
    "\" not found for class(es) ", 
    paste0(names(missing_classes), collapse = ", ")
  )
}

so that

test_that("prob method exists for all subclasses of GeneralModel", {
  expect_true(
    h_all_required_methods_exist(
      "prob", 
      "GeneralModel", 
      c("numeric", "{cls}", "Samples"),
      "ModelLogNormal"
      )
    )
})

gives

Test passed 

ModelLogNormal is the virtual superclass of models with a bivariate Normal prior and a reference dose. It should never be directly instantiated.

An alternative approach would be to require all methods to exist even for virtual superclasses. The actual method implementation for these virtual superclasses would then throw an exception (as is done by the default constructors for these super classes. This would simplify the unit testing, but at the cost of additional code that is never expected to be used.

There is an additional, small, complication. When two or more classes appear in the method's signature, the possibility of small variations between subclass implementations exists. I would expect the following test to fail

test_that("fit method exists for all subclasses of GeneralModel", {
  expect_true(
    h_all_required_methods_exist(
      "fit", 
      "GeneralModel", 
      c("Samples", "{cls}", "Data"),
      "ModelLogNormal"
    )
  )
})

because fit-Samples-LogisticLogNormalOrdinal-DataOrdinal exists, but fit-Samples-LogisticLogNormalOrdinal-Data should not.
This is probably related to #733.

The exceptions parameter is designed to deal with this situation:

test_that("fit method exists for all subclasses of GeneralModel", {
  expect_true(
    h_all_required_methods_exist(
      "fit", 
      "GeneralModel", 
      c("Samples", "{cls}", "Data"),
      "ModelLogNormal",
      list("LogisticLogNormalOrdinal" = c("Samples", "{cls}", "DataOrdinal"))
    )
  )
})

passes as expected.

Questions

• If we adopt this testing strategy
  • Where should h_all_required_methods_exist be defined? We currently have class-specific helper files in testthat, but no generic helper file. So tests/testthat/helpers.h seems a sensible option.
  • Where should the tests for existence be performed? In the existing test_<class>-Methods file or, perhaps, in a new test-required-methods-exist file? I have no strong preference. If the latter, the helper function could be defined there as well?