testthat icon indicating copy to clipboard operation
testthat copied to clipboard

Move callr and friends to Suggests

Open lionel- opened this issue 3 years ago • 10 comments

To make webR support easier until we figure out callr.

lionel- avatar Mar 29 '22 11:03 lionel-

This will probably break packages that have

Config/testthat/parallel: true

on CRAN.

gaborcsardi avatar Mar 29 '22 11:03 gaborcsardi

An alternative would be to qualify all calls with :: and then they are not loaded with testthat. If it is possible to install ps, processx and callr, then this can work, and it does not break anything.

gaborcsardi avatar Mar 29 '22 11:03 gaborcsardi

An alternative would be to qualify all calls with :: and then they are not loaded with testthat. If it is possible to install ps, processx and callr, then this can work, and it does not break anything.

Which is already the case AFAICT:

❯ R -q --vanilla -e 'library(testthat); loadedNamespaces()'
> library(testthat); loadedNamespaces()
 [1] "compiler"  "magrittr"  "R6"        "cli"       "graphics"  "utils"
 [7] "grDevices" "stats"     "datasets"  "methods"   "brio"      "testthat"
[13] "rlang"     "base"

gaborcsardi avatar Mar 29 '22 11:03 gaborcsardi

It is currently not possible to install callr, ps, and processx. It might be possible without too many changes but I haven't investigated it yet. At the moment I am trying to make testthat work (it causes webR to freeze).

This will probably break packages that have Config/testthat/parallel: true

Could we fall back to serial testing if callr is not installed?

lionel- avatar Mar 29 '22 12:03 lionel-

Could we fall back to serial testing if callr is not installed?

Maybe, but they are not interchangeable.

In general it would be better to have a solution that does not break people's current workflows. Which is possible, I think. E.g. we can change processx etc. to install cleanly in webR, and fail for the function calls.

If you don't want to deal with this now, I guess you can use your testthat branch that does not have callr?

gaborcsardi avatar Mar 29 '22 12:03 gaborcsardi

yes there is no hurry, webr-repo currently uses this branch.

lionel- avatar Mar 29 '22 12:03 lionel-

Btw. there is this thing: https://browsix.org/ that handles fork(), etc. on top of web workers, so I guess at some point callr etc. will work in webassembly.

gaborcsardi avatar Mar 29 '22 12:03 gaborcsardi

yup but browsix is basically unmaintained and unused. The WASI folks seem interested in process launching and IPC the long term but there is currently not much interest in the wasm community, so we're probably years away of a working solution. It seems the best approach would be to specially handle the case of calling back to R by instantiating a new emscripten module in a web worker, but it's not clear how we can make them share the same virtual filesystem.

lionel- avatar Mar 29 '22 12:03 lionel-

Btw. if you set the TESTTHAT_PARALLEL env var to FALSE then you probably don't need a custom testthat branch. You could set it at startup I guess?

gaborcsardi avatar Mar 31 '22 07:03 gaborcsardi

This branch fixes issues at install time rather than runtime: https://github.com/lionel-/webr-repo/blob/7f0c7bb7337b9071bf3952cb8e779b52b91f201a/install.R#L6

lionel- avatar Mar 31 '22 07:03 lionel-