extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
186 stars 27 forks source link

Improve package generation & compilation tests on CI #77

Closed Ilia-Kosenkov closed 2 years ago

Ilia-Kosenkov commented 3 years ago

In light of #67 we need to modify the testing workflow. https://github.com/extendr/rextendr/blob/main/.github/workflows/test_pkg_gen.yaml We do not test two major features, which were a source of pain in #67:

  1. Non-trivial path. It is easy to call use_extendr() and document(), assuming path = ".", and having package root as the work directory. Everything works naturally. However, as soon as this changes, path resolution can break. I believe we pass path around correctly, but with future modifications, we need to track and verify this. The test script should, for example, setwd to one of the parent directories of the package root and then invoke use_extendr and document with explicit relative paths. Successful compilation ensures all paths are resolved correctly.
  2. Package re-compilation. We document() test package and then run R CMD check to verify if the package is compiled and documented correctly. https://github.com/extendr/rextendr/blob/74817048d578c640e77df740e1fa00071320a011/.github/workflows/test_pkg_gen.yaml#L119-L125 What we do not test is re-compilation when Rust code gets modified. We need to be able to modify lib.rs, adding another {roxygen2} tag (e.g. @returns), and changing the returned value to a different string, then running document() and testing if everything has been updated in one go. This includes hello_world.Rd having new entry and a differrent return value of testpkg::hello_world().

I doubt this can be achieved using R CMD check, perhaps we should perform all checks manually and throw errors if something goes wrong (e.g., with stop). Rscript interrupted with stop should return a non-zero exit code, which should fail the test job.

yutannihilation commented 3 years ago

For the first point, might it be safer to always assume path is "." and stop when the assumption is not true? (edit: apparently, I forgot about non-package usages like rust_source()... Please ignore, sorry)

Ilia-Kosenkov commented 2 years ago

This has been resolved in the past.