Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3.3.0 breaks #391

Closed
mattdowle opened this issue Aug 8, 2019 · 3 comments
Closed

v3.3.0 breaks #391

mattdowle opened this issue Aug 8, 2019 · 3 comments

Comments

@mattdowle
Copy link

covr is always so reliable and awesome! Today hit a very rare problem.
Seems to have started to fail on Travis for data.table with the recent update. Is it data.table-only or affecting more packages? Haven't debugged it yet. Thought I'd see if you were aware first.
I can reproduce it locally :

> library(covr)
> packageVersion("covr")
[1] ‘3.2.1> package_coverage()    # ok with v3.2.1
data.table Coverage: 98.47%
... snip ...
R/xts.R: 100.00%

> library(covr)
> packageVersion("covr")
[1] ‘3.3.0> package_coverage()  # not ok
files differ in number of lines:
31,33d30
<    a
< 1: 1
< 2: 4

files differ in number of lines:
36,42d35
< ```
< ##    x y  z
< ## 1: 1 4 10
< ## 2: 2 5 11
< ## 3: 3 6 12
< ```
< 
@jimhester
Copy link
Member

jimhester commented Aug 8, 2019

I can reproduce the error, 3.3.0 had a regression that caused the visibly of returned arguments to change, e.g. f(1) would normally be invisible, because having an assignment as the last expression makes the value implicitly invisible.

f <- function(x) {
  x <- 1
}

However covr 3.3.0 it would convert that call to essentially

f <- function(x) {
  ({ x <- 1 })
}

Which would then print, as () makes its value visible. 89029d4 fixes this by using identity() rather than ().

f <- function(x) {
  identity({ x <- 1 })
}

Which preserves the original visibility.

data.table has tests to verify that DT = data.table(x=1:3, y=4:6) does not print, as it is an assignment, and this was failing due to the above issue.

I will submit a new release of covr to CRAN early next week with this fix. In the meantime you can use the development version on travis etc, by putting Remotes: r-lib/covr in your DESCRIPTION file.

@mattdowle
Copy link
Author

Wow - thank's for the awesome reply, quick fix, and workaround too!

@mattdowle
Copy link
Author

mattdowle commented Aug 9, 2019

I tried adding Remotes: r-lib/covr in DESCRIPTION. Travis passed but it still used the 3.3.0 version of covr, so the coverage part still failed. My guess is that was because covr was named in the r_packages section of our travis.yml. My guess is that since it already had covr from CRAN before it got to the Remotes: part, it already had the dependency satisfied and didn't upgrade to the GitHub version.

Anyway, what did work was using r_github_packages: r-lib/covr. See this commit. Coverage working and updating again. Yay! Doing it this way also avoids the --as-cran note about the Remotes: field too. The workaround being in travis.yml only is nice and means a comment works there too (TIL that comments don't work in DESCRIPTION file!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants