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

Fix CRAN testing issues with R-devel #154

Merged
merged 2 commits into from Dec 21, 2018

Conversation

jayqi
Copy link
Collaborator

@jayqi jayqi commented Dec 20, 2018

This PR resolves two issues we had with CRAN testing our 0.3.0 submission.


test_x64 for some Windows cases

  • Added case for test_x64 to .BuildTestLib for some Windows cases

We had the following ERROR in our 0.3.0 submission to CRAN for Windows.

This is because we aren't catch the test_x64 case, which seems to be when Windows is using 64-bit R and explicitly calling it out in the directory names.

From R CMD check:

  Warning: invalid package 'd:/RCompile/CRANincoming/R-devel/pkgnet.Rcheck/tests_x64//baseballstats'
  Warning: invalid package 'd:/RCompile/CRANincoming/R-devel/pkgnet.Rcheck/tests_x64//sartre'
  Warning: invalid package 'd:/RCompile/CRANincoming/R-devel/pkgnet.Rcheck/tests_x64//milne'
  Warning: invalid package 'd:/RCompile/CRANincoming/R-devel/pkgnet.Rcheck/tests_x64'
  Error: ERROR: no packages specified

Updating .BuildTestLib to use R.home

Below is a reproduced error that we also got from CRAN testing on R-devel.

  * installing *source* package ‘milne’ ...
  ** R
  ** byte-compile and prepare package for lazy loading
  Error in rbind(info, getNamespaceInfo(env, "S3methods")) :
    number of columns of matrices must match (see arg 2)
  ERROR: lazy loading failed for package ‘milne’
  * removing ‘/tmp/RtmpMTUoXD/milne’
  Error in pkgnet:::.BuildTestLib(targetLibPath = Sys.getenv("PKGNET_TEST_LIB")) :
    Installation of packages in .BuildTestLib failed! (exit code = 1)

  WARNING: ignoring environment value of R_HOME ...  ...
  In addition: Warning message:
  In system(command = cmdstr, intern = TRUE) :
    running command '"/usr/bin/R" CMD INSTALL -l "/tmp/RtmpMTUoXD" /RPackage/pkgnet/pkgnet.Rcheck/00_pkg_src/pkgnet/inst/baseballstats /RPackage/pkgnet/pkgnet.Rcheck/00_pkg_src/pkgnet/inst/sartre /RPackage/pkgnet/pkgnet.Rcheck/00_pkg_src/pkgnet/inst/milne /RPackage/pkgnet/pkgnet.Rcheck/00_pkg_src/pkgnet' had status 1
  Execution halted

I'm not entirely sure what the problem was. I was able to reproduce this error just straight up R CMD INSTALL-ing milne on the R-devel docker image, though RD CMD INSTALL did not have any problem. This made me realize that .BuildTestLib is hard-coded to use R even for RD CMD CHECK.

Instead, we use file.path(R.home("bin"), "R") to look up the binary for the current R process. So if you call from an RD process, you will get the correct path to the r-devel binary.

https://stackoverflow.com/questions/33798115/command-to-see-r-path-that-rstudio-is-using

@codecov-io
Copy link

codecov-io commented Dec 20, 2018

Codecov Report

Merging #154 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #154     +/-   ##
=========================================
+ Coverage   84.55%   84.66%   +0.1%     
=========================================
  Files          10       10             
  Lines         913      913             
=========================================
+ Hits          772      773      +1     
+ Misses        141      140      -1
Impacted Files Coverage Δ
R/testing_utils.R 80% <100%> (+2.85%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1a4a9fe...d3e5cfa. Read the comment docs.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jayqi jayqi changed the title Added case for test_x64 to .BuildTestLib for some Windows cases Fix CRAN testing issues with R-devel Dec 21, 2018
# This guarantees you're using the right R binary, e.g., in case someone
# from CRAN is running RD CMD CHECK instead of R CMD CHECK
# https://stackoverflow.com/questions/33798115/command-to-see-r-path-that-rstudio-is-using
R_LOC <- file.path(R.home("bin"), "R")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@bburns632 bburns632 merged commit 88b348d into uptake:master Dec 21, 2018
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

Successfully merging this pull request may close these issues.

None yet

4 participants