IQSS / dataverse-client-r

R Client for Dataverse Repositories
https://iqss.github.io/dataverse-client-r
61 stars 25 forks source link

switch back from `\donttest{}` to `\dontrun{}` #92

Closed wibeasley closed 3 years ago

wibeasley commented 3 years ago

from @kuriwaki in https://github.com/IQSS/dataverse-client-r/commit/f88a01c34a1bb8673208ae49120de7d5225b8a1d#commitcomment-47749130

@wibeasley - studying the Rhub Solaris error, I see that the problematic "donttest{" call is used only once in the entire package (where the example error occurs), whereas "dontrun{" is used dozens of times. Is this intentional?

Seems the "\donttest" part all ran in the tests, and only in Solaris was there a problem.  Since R 4.0.0, R CMD check --as-cran runs \donttest examples by default. (https://community.rstudio.com/t/r-cmd-check-r-4-0-0-now-runs-donttest-how-to-proceed-with-long-running-examples/64493/3)


@kuriwaki, looks like you're right again. I'm sure I had a reason to change it the first time. Maybe I was looking at the first segment instead of the second?

from https://r-pkgs.org/r-cmd-check.html?q=donttest#documentation

Checking examples. Every documentation example must run without errors, and must not take too long. Exclude failing or slow tests with \donttest{}. See documenting functions for more details.

from https://r-pkgs.org/man.html?q=donttest#man-functions

For the purpose of illustration, it’s often useful to include code that causes an error. \dontrun{} allows you to include code in the example that is not run. (You used to be able to use \donttest{} for a similar purpose, but it’s no longer recommended because it actually is tested.)

I think my real motivation was an unsuccessful attempt to have the output included in the pkgdown website, but not be a potential point of failure for CRAN. My next commit was to update the site.

It's frustrating that all these pass on our local Ubuntu, Windows, & Mac machines (and Travis & win-builder), but fail on a subset of CRAN builds. Looking at the CRAN errors again, I think you're totally right that this is the problem because it was complaining about the pillar package (and later its dependency the utf8 package). The colored text in the screenshot below is formatted by pillar by readr when converting the csv to a data.frame.

image

wibeasley commented 3 years ago

Changing this change doesn't eliminate the problems on r-hub if the package dependency is needed by the vignettes. https://builder.r-hub.io/status/dataverse_0.3.6.tar.gz-479a0cbd90164faba6988884ffea0e36

Here's an error thrown because utf8 is currently broken on the two windows cran builds. This dependency is so deep within the tidyverse packages (I think it's a 3rd layer dependency from dataverse) that it's gotta be causing a lot of problems for a lot of packages. I imagine it will be fixed soon.

image

R-hub output:

812#> ERROR 813#> Running the tests in 'tests/testthat.R' failed. 814#> Last 13 lines of output: 815#> 13. -base::loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) 816#> 14. +-base::namespaceImportFrom(...) 817#> 15. | -base::asNamespace(ns) 818#> 16. -base::loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) 819#> 17. +-base::namespaceImportFrom(...) 820#> 18. | -base::asNamespace(ns) 821#> 12. | -base::asNamespace(ns) 822#> 19. -base::loadNamespace(j <- i[[1L]], c(lib.loc, .libPaths()), versionCheck = vI[[j]]) 823#> 21. -base:::withOneRestart(expr, restarts[[1L]]) 824#> 20. -base::withRestarts(stop(cond), retry_loadNamespace = function() NULL) 825#> 22. -base:::doWithOneRestart(return(expr), restart) 826#> 827#> [ FAIL 24 | WARN 0 | SKIP 0 | PASS 52 ] 828#> Error: Test failures 829#> Execution halted 830#> checking for unstated dependencies in vignettes ... OK 831#> checking package vignettes in 'inst/doc' ... OK 832#> * checking re-building of vignette outputs ... WARNING 833#> Error(s) in re-building vignettes: 834#> ... 835#> --- re-building 'A-introduction.Rmd' using rmarkdown 836#> --- finished re-building 'A-introduction.Rmd' 837#> --- re-building 'B-search.Rmd' using rmarkdown 838#> --- finished re-building 'B-search.Rmd' 839#> --- re-building 'C-download.Rmd' using rmarkdown 840#> Quitting from lines 55-60 (C-download.Rmd) 841#> Error: processing vignette 'C-download.Rmd' failed with diagnostics: 842#> there is no package called 'utf8' 843#> --- failed re-building 'C-download.Rmd' 844#> SUMMARY: processing the following file failed: 845#> 'C-download.Rmd' 846#> Error: Vignette re-building failed.

kuriwaki commented 3 years ago

Thanks for this explanation. My vignettes still compile on local if %\VignetteEncoding{UTF-8} is removed, but I see other packages have that so maybe best not to fiddle with that.

wibeasley commented 3 years ago

Yea, and fwiw, I don't think the utf8 package is related to the vignette syntax %\VignetteEncoding{UTF-8}.