covr icon indicating copy to clipboard operation
covr copied to clipboard

Feature Request: Recording srcrefs of test expressions & link to covr traces

Open dgkf opened this issue 4 years ago • 3 comments

It would be super helpful if, in addition to the srcrefs of the code that gets executed by each test, we could also record the srcrefs of the unit tests that trigger their evaluation.

Motivation

I'm helping to develop an R package validation strategy, and one feature that would be tremendously valuable would be the ability to link individual unit tests to the traces that execute during the test execution. By linking the annotated test behavior to the trace that is executed, we can automate the construction of a traceability matrix, an expected part of any validation report.

Work so far

I've built up a proof of concept in a fork at dgkf/covr@dev/testrefs. However, to keep changes minimal, I've recorded a list of srcrefs (called testrefs) associated with the test for each and every trace (so that the length of $testrefs will be equal to $value).

covr_result <- covr::package_coverage("./example")
covr_result[[1]]
# $value 
# [1] 1
#
# $srcref
# return("example result")
#
# $functions
# [1] "example"
#
# $testrefs
# $testrefs[[1]]
# test_that("example function communicates wip structure.", {
#   expect_silent(example())
# })

In my benchmarks, I found that execution time is minimally affected (~5% overhead). However, one big limitation of this approach is how much redundant information is recorded. Each and every call in the stack is recording a copy of the test trace. This results in quite large coverage objects (~50-100x tmp file size, and coverage object.size). There are certainly ways of collecting this data more thoughtfully, but it would affect more of the covr codebase.

Proposed Changes to coverage Data Structure

If there's interest in including this additional data, I would propose a data structure that

  1. Keeps existing data in the same place to minimize impact to existing covr code
  2. Adds a list of test srcrefs as an attribute to the coverage object (attr(,"tests"))
  3. Add a tests matrix to each item within the coverage object list, which has a row per execution of the trace and columns for test number (corresponding to index in the test srcrefs attributes) and stack depth into the target package during execution (allowing differentiation of calls directly from the test from those that are called from within the package).
> unclass(covr_result)
# $example.R:1:2:3:4:5:6:7:8
# $`example.R:1:2:3:4:5:6:7:8`$value
# [1]  1
# $`example.R:1:2:3:4:5:6:7:8`$srcref
# return("example result")
#
# $`example.R:1:2:3:4:5:6:7:8`$functions
# [1] "example"
#
# $`example.R:1:2:3:4:5:6:7:8`$tests
#       test depth
# [1,]     1     1
#
# attr(,"tests")  # list of test expression srcrefs
# [[1]]
# test_that("example function communicates wip structure.", {
#   expect_silent(example())
# })
#
# attr(,"package")  # other attributes already part of coverage objects
# ...
#
# attr(,"relative")
# ...

I'm happy to help implement the changes, but wanted to gauge interest in adding this functionality before making changes that might affect more of the codebase.

dgkf avatar Jan 19 '21 19:01 dgkf

Sure if this would be useful to you I would be happy for the contribution. However we need to makes sure that is an option and is off by default. As long as as it is optional I think it is ok if it is a lot of data.

jimhester avatar Jan 20 '21 15:01 jimhester

Sounds good, @jimhester - I'll do my best to implement this structure and will include an option to enable the off-by-default behavior. If I change the structure of the underlying coverage objects, are there functions that I should pay close attention to?

To my knowledge, trace_calls & count will need to be updated to gather the appropriate data, and merge_coverage will need to be updated to also merge attr(,"tests") and the tests matrices of each trace. Are there other affected functions that should be on my radar?

dgkf avatar Jan 20 '21 18:01 dgkf

I think that is mostly it!

jimhester avatar Jan 20 '21 18:01 jimhester