diffdf icon indicating copy to clipboard operation
diffdf copied to clipboard

custom expectation for testthat

Open kieranjmartin opened this issue 4 years ago • 3 comments

A nice integration might be to have something like expect_equal_df for testthat. This could generate nice error messages for the testing reportt

kieranjmartin avatar Mar 09 '21 16:03 kieranjmartin

this is actually a neat idea....

My only concern would then be though that it requires adding test that to the dependencies which is super not light

image

I guess we could include it as a suggests which could limit that somewhat ... ?

gowerc avatar Mar 15 '21 11:03 gowerc

After raising this to @kieranjmartin I actually implemented this for one of my packages recently. The code is fairly simple.

#' Expectation: Are Two Datasets Equal
#'
#' Uses [diffdf::diffdf()] to compares 2 datasets for any differences
#'
#' @param base Input dataset
#' @param compare Comparison dataset
#' @param keys `character` vector of variables that define a unique row in the
#'        base and compare datasets
#' @param ... Additional arguments passed onto [diffdf::diffdf()]
#'
#' @examples
#' testthat::test_that("a missing row is detected", {
#'   data(dm)
#'   expect_dfs_equal(dm, dm[-1L, ], keys = "USUBJID")
#' })
#'
expect_dfs_equal <- function(base, compare, keys, ...) {
  diff <- diffdf::diffdf(base, compare, keys, suppress_warnings = TRUE, ...)
  if (diffdf::diffdf_has_issues(diff)) {
    msg <- capture.output(print(diff))
    testthat::fail(msg)
  } else {
    testthat::succeed()
    invisible()
  }
}

I'd be happy to open a pull request for this.

Regarding the {testthat} dependency I would indeed add it to Suggests rather than Imports.

thomas-neitmann avatar Mar 30 '21 09:03 thomas-neitmann

I was thinking about this the other day a bit more and I can't make up my mind on it.

my main concern is does it provide enough value to be worth including testthat as a dependency. I have a strong preference to keep the dependency list as small as possible. Realistically I'm not sure I'm convinced this then adds enough utility. Like the test is already possible with exepect_equal() the only difference is the error message which I would expect users to manually use to diagnose.

Though having said that I guess this is a nice way of getting diffdf to throw an error without suppressing the comparison information. hmmmm @kieranjmartin any thoughts ?

gowerc avatar Apr 01 '21 21:04 gowerc