covr considers only the first instance of expect_snapshot in a test

moodymudskipper commented 3 years ago

This can be reproduced with :

package code :

foo <- function(cond) {
  if(cond) {
    print("TRUE!") }
  else  {
    print("FALSE!")
  }
}

tests :

test_that("foo works", {
  expect_snapshot({
    foo(TRUE)
  })

  expect_snapshot({
    foo(FALSE)
  })
})

output :

The following would work fine :

test_that("foo works", {
  expect_snapshot({
    foo(TRUE)
    foo(FALSE)
  })
})

Thanks for the great work.

jimhester commented 3 years ago

It works outside a package, I think this must be some sort of incompatibility with testthat in how it is executing the snapshot tests in a package.

code <- 'foo <- function(cond) {
  if(cond) {
    print("TRUE!") }
  else  {
    print("FALSE!")
  }
}'

tests <- '
library(testthat)
test_that("foo works", {
  local_edition(3)
  expect_snapshot({
    foo(TRUE)
  })

  expect_snapshot({
    foo(FALSE)
  })
})
'

cov <- covr::code_coverage(code, tests)
#> Can't compare snapshot to reference when testing interactively
#> ℹ Run `devtools::test()` or `testthat::test_file()` to see changes
#> ℹ Current value:
#> Code
#>   foo(TRUE)
#> Output
#>   [1] "TRUE!"
#> Can't compare snapshot to reference when testing interactively
#> ℹ Run `devtools::test()` or `testthat::test_file()` to see changes
#> ℹ Current value:
#> Code
#>   foo(FALSE)
#> Output
#>   [1] "FALSE!"
#> Test passed 🥳
cov
#> Coverage: 100.00%
#> /var/folders/9x/_8jnmxwj3rq1t90mlr6_0k1w0000gn/T//RtmpVWIv39/source.R53fe668bf045: 100.00%
as.data.frame(cov)
#>                                                                            filename
#> 1 /var/folders/9x/_8jnmxwj3rq1t90mlr6_0k1w0000gn/T//RtmpVWIv39/source.R53fe668bf045
#> 2 /var/folders/9x/_8jnmxwj3rq1t90mlr6_0k1w0000gn/T//RtmpVWIv39/source.R53fe668bf045
#> 3 /var/folders/9x/_8jnmxwj3rq1t90mlr6_0k1w0000gn/T//RtmpVWIv39/source.R53fe668bf045
#>   functions first_line first_byte last_line last_byte first_column last_column
#> 1       foo          2          6         2         9            6           9
#> 2       foo          3          5         3        18            5          18
#> 3       foo          5          5         5        19            5          19
#>   first_parsed last_parsed value
#> 1            2           2     2
#> 2            3           3     1
#> 3            5           5     1

^{Created on 2021-06-21 by the reprex package (v2.0.0)}

Maybe @hadley can comment on what might be the issue, I don't know how testthat executes snapshot tests internally.

jimhester commented 3 years ago

https://github.com/jimhester/snapshot is a repo to reproduce this behavior by running covr::report() or covr::package_coverage()

assignUser commented 2 years ago

@jimhester It looks like this is caused by some behavior change in testthat when on CRAN as the coverage is detected correctly when NOT_CRAN is set to true (and it has to be lowercase TRUE does not work).

~/R/snapshot (master|✔) $ Rscript -e "covr::package_coverage()"
snapshot Coverage: 66.67%
R/package.R: 66.67%

~/R/snapshot (master|✔) $ Rscript -e "Sys.setenv(NOT_CRAN = 'true'); covr::package_coverage()"
snapshot Coverage: 100.00%
R/package.R: 100.00%

~/R/snapshot (master|✔) $ Rscript -e "Sys.setenv(NOT_CRAN = 'TRUE'); covr::package_coverage()"
snapshot Coverage: 66.67%
R/package.R: 66.67%

VincentGuyader commented 5 months ago

Thank you for these investigations. We just stumbled upon this issue today. I see that a merge request has been accepted, but indeed the issue is still present.

hadley commented 5 months ago

Interestingly devtools:: test_coverage() works correctly — I see 100% coverage for https://github.com/jimhester/snapshot. It's not clear to me what code in that function makes it work, but I suspect its testthat::local_test_directory().

hadley commented 5 months ago

Oooh, as @assignUser points out, testthat doesn't run snapshot tests on CRAN by default.

r-lib / covr

covr considers only the first instance of expect_snapshot in a test #482