unitize_dir does not restore working directory on quit in package development mode

[blset]

under rstudio
inside an roci package development project

reset R session
devtools:load_all()
library(unitizer)
unitize_dir(state = 'recommended')
then Q at the unitize prompt (correct tests unitize dir tests ok)

Error in library(name, pos = pos, quietly = TRUE, character.only = TRUE, :
‘roci’ is not a valid installed package
Warning in reattach(i, name = obj.name.clean, type = obj.type, data = search.target$search.path[[i]], :
Internal Warning: unable to fully restore search path; see prior error. Contact maintainer if this warning persists.

the error disappear if I fully install and restart the package (from rstudio button in the build pane)

but since this is a tool for development...

[brodieG]

Thanks for reporting. Honestly I hadn't even considered this corner case, where there is an environment in the search path masquerading as a package environment, but is not really a package environment.

This could either be real easy to address, or possibly very difficult, depending on how the environment is built. I'll look at it at some point because I do agree that it would be nice if this worked, but what we're dealing here is the intersection of a hack (what devtools::load_all does), with another hack (what unitizer does to manage the search path) so I'll probably only address this if it is straightforward to deal with.

In the meantime I think you have two options:

1. use state='default' and don't let unitizer mess with the search path.
2. actually install your package (see below).
3. add 'package::roci' to the unitizer.search.path.keep global option so that unitizer doesn't touch it.

I am a reasonably active R package developer, and test/build packages all the time, use devtools, yet almost never use devtools::load_all. I just re-install the packages either with devtools::install or with install.packages (or more generally, with a shortcut for install.packages that I keep in my .Rprofile file), and I've never found this to be a pain point in my development process. I do use load_all every now and then, and I've never found it to be noticeably faster than install so I am unclear on its benefits (note: I really dislike the idea of exporting internal funs even for testing purposes b/c then it means that the testing environment is not the same as the usage environment. I always use ::: when testing them; when I use devtools::load_all it is always with export=FALSE).

Loosely related issue: ropensci/drake#30

Finally, thanks for taking the time to take unitizer for a spin.

[blset]

If I understand well devtools::load_all is not the best option before running tests

but then devtools::test() using testthat does not restart environment neither before running the tests

so the recommended way is to systematically install /restart / load and then run the tests

finally, this is a very mild bug which better looks like a sane warning

[brodieG]

Yeah, basically you can't use devtools::load_all with state='recommended'.

My workflow is typically:

1. Write code and tests
2. install.packages('.', repos=NULL, type='source'), (or devtools::install)
3. unitize_dir
4. If there is a problem with the test:
   • Fix my code
   • run devtools::install (b/c the working directory is changed to the test directory in unitizer)
   • 'R' to re-run the tests

Note you should not need to restart R or re-load your packages (so long as you have a library(mypackage) call in your tests). Sometimes re-installing packages in R is problematic (this is an R issue, not necessarily a unitizer issue). For those packages you'll need to restart R.

[brodieG]

I looked into this further and concluded it is non-trivial to extend the search path management tools to include devtools::load_all since it itself does a fair bit of undocumented stuff. Instead, I've added documentation to highlight this, and made the failure more graceful so that e.g. the working directory is restored correctly.

These changes are now on the development branch.