IQSS / dataverse-client-r

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

CRAN check errors #123

Closed kuriwaki closed 1 year ago

kuriwaki commented 1 year ago

Two checks failing currently at https://cran.r-project.org/web/checks/check_results_dataverse.html

From CRAN:

1) persistently

Error: The API token expired on 2023-01-01, so the tests would probably fail. Please regenerate a new token and update inst/constants.yml

2) Transiently

--- re-building ‘B-search.Rmd’ using rmarkdown
 Quitting from lines 21-24 (B-search.Rmd)
 Error: processing vignette ‘B-search.Rmd’ failed with diagnostics:
 Internal Server Error (HTTP 500). Failed to Exception running

search for [Gary King] with filterQueries [dvObjectType:(dataverses OR datasets OR files), ] and paginationStart [0]: edu.harvard.iq.dataverse.search.SearchException: Internal Dataverse Search Engine Error org.apache.solr.client.solrj.SolrServerException org.apache.solr.client.solrj.SolrServerException: Timeout occurred while waiting response from server at: http://dvn-cloud-solr.lib.harvard.edu:8983/solr/collection1 java.net.SocketTimeoutException java.net.SocketTimeoutException: Read timed out . --- failed re-building ‘B-search.Rmd’

kuriwaki commented 1 year ago

For some reason the new token I got and submitted for v0.3.12 had an expiration date in March 2023, so now the tests are failing again at CRAN after only two months. CRAN gave me a deadline to fix this by March 27.

I can fix this again with a token that lasts till 2024-03-13, but I wonder if it is better to try and skip even this expiration check for CRAN, like we do for all the tests. @wibeasley what do you think? I wonder if we can put skip_on_cran() in the testthat/testthat.R root file, which is what's failing now. Even a once-a-year update now feels like a liability to keep this package on CRAN.

Log. Currently on: https://cran.r-project.org/web/checks/check_results_dataverse.html

Version: 0.3.12
Check: tests
Result: ERROR
     Running ‘testthat.R’ [1s/1s]
    Running the tests in ‘tests/testthat.R’ failed.
    Complete output:
     > library("testthat")
     > library("dataverse")
     >
     > if (!requireNamespace("yaml", quietly = TRUE)) {
     + warning("The 'yaml' package must be present to test the dataverse package.")
     + } else if (!requireNamespace("checkmate", quietly = TRUE)) {
     + warning("The 'checkmate' package must be present to test the dataverse package.")
     + } else {
     + server <- Sys.getenv("DATAVERSE_SERVER")
     + key <- Sys.getenv("DATAVERSE_KEY")
     +
     + if (server == "" | key == "") {
     + config <- yaml::read_yaml(system.file("constants.yml", package = "dataverse"))
     + # config <- yaml::read_yaml("inst/constants.yml")
     +
     + Sys.setenv("DATAVERSE_SERVER" = config$server)
     + Sys.setenv("DATAVERSE_KEY" = config$api_token)
     +
     + # To better identify the source of problems, check if the token is expired.
     + # This check *should* be unnecessary on CRAN, since not CRAN tests should
     + # try to access any server.
     + if (Sys.getenv("NOT_CRAN") %in% c("", "true")) {
     + if (as.Date(config$api_token_expiration) < Sys.Date()) {
     + stop(
     + "The API token expired on `",
     + config$api_token_expiration,
     + "`, so the tests would probably fail. ",
     + "Please regenerate a new token and update `inst/constants.yml`"
     + )
     + }
     + }
     + rm(config)
     + }
     + rm(server, key)
     +
     + message("Using Dataverse server `", Sys.getenv("DATAVERSE_SERVER"), "`.")
     +
     + test_check("dataverse")
     + }
     Error: The API token expired on `2023-03-10`, so the tests would probably fail. Please regenerate a new token and update `inst/constants.yml`
     Execution halted
wibeasley commented 1 year ago
  1. Your overall point makes sense. Of all the tests in the package, this is the only one I see that doesn't depend on hitting the demo server. https://github.com/IQSS/dataverse-client-r/blob/efdcb33e015ff2a69506158d69a5aadc30cbc757/tests/testthat/tests-get_dataframe-original-basketball.R#L7

  2. I don't understand that specific error message. The value I see is "2023-01-01" https://github.com/IQSS/dataverse-client-r/blob/099b0df896123264baa6fcbe5642e2d5f85d8ec9/inst/constants.yml#L3, which is referenced on this line the file you linked to: https://github.com/IQSS/dataverse-client-r/blob/efdcb33e015ff2a69506158d69a5aadc30cbc757/tests/testthat.R#L26 So the message about "2023-03-10" confuses me. I don't see any other instances of "2023" in the repository today: https://github.com/search?q=repo%3AIQSS%2Fdataverse-client-r+2023&type=code

kuriwaki commented 1 year ago

Thank you @wibeasley.

From the log, I think the error is coming from testthat/testthat.R https://github.com/IQSS/dataverse-client-r/blob/efdcb33e015ff2a69506158d69a5aadc30cbc757/tests/testthat.R#L19-L22 rather than a specific test function. Do you know why this part might get evaluated and throw an error in CRAN's server even though we have Sys.getenv("NOT_CRAN")?

I will also put a skip_if_cran() on standardize_string(), but that function does not use any dataverse functions.

For your point 2, I think the issue is that the version on CRAN right now is the package from the dev branch, rather than master.

wibeasley commented 1 year ago

Do you know why this part might get evaluated

No, and that's tricky to debug. It's not like we can play with different version.

I can see how CRAN changes some environmental subtly (eg, it used to be "CRAN" but now it's "cran"). But I can't see how/why something on CRAN would set "NOT_CRAN".

Oh wait, I think this comment helps:

  • skip_on_cran() skips on CRAN (using the NOT_CRAN env var set by devtools and friends).

I'm interpreting that this variable is set only if testthat/devtools is the one starting the test. So if you don't want it running on CRAN, change it to if (Sys.getenv("NOT_CRAN") == "true") {

kuriwaki commented 1 year ago

That sounds like the same as what we had (if (Sys.getenv("NOT_CRAN") %in% c("", "true"))), but this is helpful. In the dev branch in the above commit I have used identical to match it up with the testthat code, and also loaded the devtools package since from the description NOT_CRAN gets set by devtools.

I plan to re-submit it to CRAN as v.0.3.13 tomorrow. The Github Actions checks now all pass!