[GitHub] [arrow] jonkeane commented on a change in pull request #12486: ARROW-15673 [R] Error gracefully if DuckDB isn't installed

[GitBox <gi...@apache.org>]

jonkeane commented on a change in pull request #12486:
URL: https://github.com/apache/arrow/pull/12486#discussion_r812299074



##########
File path: r/tests/testthat/test-duckdb.R
##########
@@ -15,6 +15,19 @@
 # specific language governing permissions and limitations
 # under the License.
 
+test_that("meaningful error message when duckdb is not installed", {
+  new_lib <- tempfile()
+  dir.create(new_lib)
+  withr::with_libpaths(new_lib, {
+    ds <- InMemoryDataset$create(example_data)
+    expect_error(
+      to_duckdb(ds),
+      "Please install the `duckdb` package."
+    )
+  })
+  unlink(new_lib, recursive = TRUE)

Review comment:
       Nice use of `with_libpaths()` and nice that this is up at the top before duckdb might have already been loaded (we should probably also have a comment explaining that since it might be confusing to others who run across this code).
   
   `.libPaths()` is a strange beast that has some pretty peculiar behavior. I suspect what's going on here is some combination of:
   * that `new_lib` might not actually be created as a directory yet. (if you pass `.libPaths()` a string that's not a directory it silently ignores it)
   * `.libPaths()` won't overwrite or add to libraries specified in `.Library.site` or `.Library` https://stat.ethz.ch/R-manual/R-devel/library/base/html/libPaths.html
   
   You might want to try and print `.libPaths()` to confirm what you expect it to be. You also might want to use our helper `make_temp_dir()` which should ensure that a temp directory is actually created  




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org