rticulate / import

An Import Mechanism For R
https://import.rticulate.org
Other
222 stars 14 forks source link

Error when using in unit tests #76

Closed kenahoo closed 11 months ago

kenahoo commented 1 year ago

The import package looks like an excellent fit for unit tests - because one gotcha I've run into many times is that if I put something like library(lubridate) in my test scripts, that can hide problems with my package forgetting to import, say, lubridate::seconds into its environment.

My initial attempt at using it is failing, though. My test script tests/testthat/test-dard.R starts like this:

futile.logger::flog.threshold('WARN')

import::from(mockthat, mock, with_mock, mock_n_called, mock_arg, mock_args)
import::from(lubridate, ymd_hms, force_tz, with_tz, ceiling_date, hours, seconds)

When I run it (either by doing devtools::test(filter='dard'), or running all tests in RStudio using Shift-Cmd-T), I get the following error:

Error (test-dard.R:3): (code run outside of `test_that()`)
Error in `as.environment(where)`: no item called "imports" on the search list
Backtrace:
 1. import::from(...)
      at test-dard.R:3:0
 2. base::exists("?", .into, mode = "function", inherits = FALSE)

Presumably this has to do with where the imported objects are being placed - the import docs say this:

By default, imported objects are placed in a separate entity in the search path called “imports”.

but beyond that, I'm not sure how to diagnose what's going wrong.

torfason commented 1 year ago

This is a known issue with an unknown solution. When implementing unit tests for import itself, we gave up and used a convoluted method, spinning up a new R instance to do the tests. For the implementation, see:

https://github.com/rticulate/import/blob/main/tests/testthat/test_import.R

So this is unlikely to be fixed or have a good workaround. But any fix that was found would enable us to revert to normal tests, which would make us maintainers very happy.

:-)

kenahoo commented 1 year ago

Thanks for the quick response! If the issue is known but the solution is unknown, do we at least understand what's causing the problem? Or is that unknown too?

I did search through existing issues to check whether this has been reported before, and couldn't find one - happy to link to a previous ticket if one exists.

kenahoo commented 1 year ago

Also, I did actually find a decent workaround, which was to use import::here() instead of just import::from(), because that avoids creating the imports entry in the search path.

torfason commented 1 year ago

That's great to hear! It does make sense, as import::here() is much simpler with regards to environments. That unfortunately wont solve our own unit tests, because for that we need to be able to run all three functions, but I'm glad it solves it for you :-)

torfason commented 11 months ago

Sounds like the good way forward is to use import::here() when using within unit tests (in line with the recommendation for other complex situations with regards to environment), and so there is not a requirement for specific updates to the package that stems from this.