ankane / jetpack

A friendly package manager for R
Other
240 stars 16 forks source link

load renv before running tests #25

Closed kevinushey closed 2 years ago

kevinushey commented 2 years ago

This will give renv an opportunity to run some hooks on startup, which can be used to help ensure that jetpack passes tests when running reverse dependency checks for renv package submissions.

ankane commented 2 years ago

Hey @kevinushey, thanks for the PR. Can you give more context on the change? I'm worried this may cause the tests to pass even when something's broken with how Jetpack loads renv.

kevinushey commented 2 years ago

The main challenge for renv submissions is that, when reverse dependency checks are run, we are in a state where:

  1. The current version of renv, e.g. renv 0.15.3, is being used for tests,
  2. An older version of renv is on CRAN; e.g. renv 0.15.2,
  3. Therefore, tests that try to use renv which might attempt to restore the current version from CRAN can fail.

Right now, renv is working around this in a pretty hamfisted way. renv now bundles a small CRAN-like repository within the package itself, at:

system.file("repos", package = "renv")

And, if renv can detect that it's being used by jetpack during tests, it updates the R repositories so that this custom local repository is visible (so that the current version of renv is restorable).

The associated patch is here:

https://github.com/rstudio/renv/blob/d217845f2765877c4a192a4b1b4d419487db4b60/R/patch.R#L155-L196

Ideally, jetpack would be able to run its tests even without internet access; that is, the tests could be run with a local on-disk package repository. (This is the strategy that renv takes.) This is understandably tricky to get right, though.

In theory, if renv were loaded up-front by jetpack, then we could initialize this dummy repository on-demand in the R tempdir(), and reuse that repository in tests. However, this doesn't work well currently because the tests of the jetpack CLI mean that different and ephemeral R temporary directories will be used, and those might not survive the whole duration of the test suite.

ankane commented 2 years ago

Thanks, that's helpful. Ideally, we'd only need to patch during the reverse dependency check (if there's a way to detect that). Also, we can skip the CLI tests on CRAN if that helps.

kevinushey commented 2 years ago

Ideally, we'd only need to patch during the reverse dependency check (if there's a way to detect that)

For renv, you could check whether the version of R available in available.packages() matches the version of renv that's actually installed.

ankane commented 2 years ago

Was hoping there would be an environment variable to check, but don't see one.

Let's go with the approach in the PR.

kevinushey commented 2 years ago

Thank you! I really appreciate it.