rstudio / pins-r

Pin, discover, and share resources
https://pins.rstudio.com
Other
312 stars 63 forks source link

Use of `is_testing()` is a problem for folks who program against pins functions #640

Closed juliasilge closed 1 year ago

juliasilge commented 2 years ago

Interesting - as I was writing more of my unit tests, I noticed that the pin_write function behaves differently when it is run within or outside unit tests (of the testthat variety). That's because the time stamp part of the version is fixed in a unit test:

https://github.com/rstudio/pins-r/blob/9e8c08123861eeef4853caa5ffd1970c9be0457e/R/pin_versions.R#L146

I didn't expect that, and it took me a good while to figure out why my unit tests were failing. (I was writing the same file twice into a versioned board. My project includes tracking the metadata in a SQL database - and that complained about duplicated version keys.)

There is nothing intrinsically wrong with the current approach, but it reminds me a bit of the Volkswagen scandal 😄 and the behavior definitely was surprising. (I solved it by temporarily unsetting the TESTTHAT environmental variable in my test.)

Originally posted by @tomsing1 in https://github.com/rstudio/pins-r/issues/637#issuecomment-1220037880

juliasilge commented 2 years ago

The current use of is_testing(), for example, in version_name(): https://github.com/rstudio/pins-r/blob/9e8c08123861eeef4853caa5ffd1970c9be0457e/R/pin_versions.R#L147-L148 makes it hard for folks to develop against pins functions and write their own tests.

juliasilge commented 1 year ago

The new tests for write_board_manifest() (specifically the ones in test-board_folder.R that look at the structure of the manifest file) involve versions and could be improved in terms of how easy they are to grasp. Currently, we use is_testing() to handle versions in tests but we don't want to continue doing that.

One option for some cases might be to use snapshot testing with a function to redact versions.

juliasilge commented 1 year ago

We could switch to use some mocking, like this:

library(pins)
b <- board_temp()

## pin_write() calls version_name() internally
b %>% pin_write(1:10, name = "nice-numbers", type = "json")
#> Creating new version '20230301T003852Z-c3943'
#> Writing to pin 'nice-numbers'

library(mockery)

# call to version_name() is 2 layers down from pin_store()
stub(
  pin_store, 
  "version_name", 
  "20120304T050607Z-xxxxx",
  depth = 2
  )

## now with stub:
b %>% pin_write(1:10, name = "nice-numbers", type = "json")
#> Replacing version '20230301T003852Z-c3943' with '20120304T050607Z-xxxxx'
#> Writing to pin 'nice-numbers'

Created on 2023-02-28 with reprex v2.0.2

Notice that the new version name has been stubbed out.

github-actions[bot] commented 1 year ago

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.