r-dbi / DBItest

Testing DBI drivers
http://dbitest.r-dbi.org
GNU Lesser General Public License v2.1
24 stars 18 forks source link

Workflow for diagnosing and fixing test failures #165

Closed hadley closed 4 years ago

hadley commented 6 years ago

I think DBItest needs a write up (and some improved tooling) that describes what you should do when a DBItest fails - i.e. how to do you go from a failing test to a minimal reprex that you can rapidly iterate with to fix the problem.

One first step would be to expose an only argument which would be the inverse of skip. Then you could (e.g.) call DBItest::test_driver(only = "data_type_driver") in the console.

krlmlr commented 6 years ago

We already have DBItest::test_some().

I think what we really need is better meta-programming: code that rewrites tests in a form like

con <- dbConnect()
dbDoSomething(con, ...)
dbDisconnect(con)

that is easy to run from a dedicated script. (Does that make sense?)

hadley commented 6 years ago

I don't think DBItest needs more complexity; it needs less. The place to start would be a written guide that (e.g.) discusses test_some()

krlmlr commented 6 years ago

There is a vignette, but it's not comprehensive: http://dbitest.r-dbi.org/articles/test.html.

I see conflicting goals:

  1. align the tests with the written specification
  2. don't repeat yourself
  3. understand the reason of a test failure

DBItest is currently biased to support 1. and 2. well, but not so much 3.. Other options are:

Meta-programming can solve all three goals at once: the spec describes a code generator that creates many simple tests, these tests are then run. If roxygen2 supported "true" literate programming (documentation chunks can appear in arbitrary order in the source files), we could refactor many tests.

krlmlr commented 6 years ago

I agree that debugging test failures is currently very hard and not pleasant. There's no easy way to set a breakpoint in the test suite (other than editing the code and recompiling), and it's hard to trace a particular failure to the source of the problem.

Expanding on the idea of generating many simple tests: DBItest could generate test files (in tests/testthat or just tests), much the same way as roxygen2 generates .Rd files. Then, the workflow would be:

  1. Run DBItest::generate_tests() on the package.
  2. Run devtools::test() against code generated by DBItest.
  3. If failures occur, the generated code is simple enough to step through manually. The DebugReporter can be used to halt execution at the first failure.
krlmlr commented 5 years ago

rlang's call traces may help. Other than that, I think a good short-term solution would be to record all DBI interface calls using a proxy, and logging them (in the form of runnable R code). Using deparse() first, datapasta can help make this code easier to read.

hadley commented 5 years ago

Again, your proposed solution make things more complex. I think that is the wrong direction to be travelling in.

krlmlr commented 5 years ago

As discussed, working on a proof of concept. Capturing calls works so far:

library(rlang)

print_call <- function(...) {
  args <- list(...)

  call <- eval_tidy(quo(call("print_call", !!!args)))
  print(styler::style_text(deparse(call, width.cutoff = 80)))
  invisible(call)
}

print_call()
#> print_call()
print_call(1, 2:3, a = 4:6 + 0, Sys.Date(), b = Sys.time())
#> print_call(1, 2:3, a = c(4, 5, 6), structure(17951, class = "Date"), b = structure(1551033819.57973, class = c(
#>   "POSIXct",
#>   "POSIXt"
#> )))
call <- print_call(iris[1:3, ])
#> print_call(structure(list(Sepal.Length = c(5.1, 4.9, 4.7), Sepal.Width = c(
#>   3.5, 3,
#>   3.2
#> ), Petal.Length = c(1.4, 1.4, 1.3), Petal.Width = c(0.2, 0.2, 0.2), Species = structure(c(
#>   1L,
#>   1L, 1L
#> ), .Label = c("setosa", "versicolor", "virginica"), class = "factor")), row.names = c(
#>   NA,
#>   3L
#> ), class = "data.frame"))
call
#> print_call(list(Sepal.Length = c(5.1, 4.9, 4.7), Sepal.Width = c(3.5, 
#> 3, 3.2), Petal.Length = c(1.4, 1.4, 1.3), Petal.Width = c(0.2, 
#> 0.2, 0.2), Species = c(1L, 1L, 1L)))

Created on 2019-02-24 by the reprex package (v0.2.1.9000)

krlmlr commented 5 years ago

Some tests require two connections to the database, these can be told apart with identical(). Serializations are stable too, but collide:

library(DBI)
con1 <- dbConnect(RSQLite::SQLite())
con2 <- dbConnect(RSQLite::SQLite())
identical(con1, con1)
#> [1] TRUE
identical(con1, con2)
#> [1] FALSE

digest::sha1(serialize(con1, NULL))
#> [1] "9eb8e8e0eb471063f229cf2a05e093a7eac469be"
digest::sha1(serialize(con1, NULL))
#> [1] "9eb8e8e0eb471063f229cf2a05e093a7eac469be"
digest::sha1(serialize(con2, NULL))
#> [1] "9eb8e8e0eb471063f229cf2a05e093a7eac469be"

Created on 2019-02-24 by the reprex package (v0.2.1.9000)

krlmlr commented 5 years ago

Printing calls works for dbConnect() and dbDisconnect().

External pointers can't be deparsed, but this seems to affect only the first argument of each generic. Need to build dictionaries for known connection and result set objects, and assign them to variables.

krlmlr commented 5 years ago

I now have this in a local tweak of RKazam. Proceeding with the implementation in DBItest:

library(RKazam)

drv <- Kazam(RSQLite::SQLite())
#> drv1 <- RSQLite::SQLite()

conn <- dbConnect(drv, file = ":memory:")
#> conn1 <- dbConnect(drv1, file = ":memory:")
dbDisconnect(conn)
#> dbDisconnect(conn1)
#> [1] TRUE

Created on 2019-02-25 by the reprex package (v0.2.1.9000)

github-actions[bot] commented 3 years ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.