search

ffverse / ffscrapr

R API Client for Fantasy Football League Platforms
https://ffscrapr.ffverse.com
Other
82 stars 21 forks source link

Refactor tests #383

Closed hadley closed 1 year ago

hadley commented 1 year ago

This was supposed to be a minimal patch to fix a failure with dev purrr, but I got a bit distracted by the challenge of running individual tests interactive, so made a couple of changes:

With that done, I still see two failing tests:

Error (test-ff_draftpicks.R:13): ff_draftpicks returns a tibble of draft picks
Error in `list2(...)`: object 'current_picks' not found

I don't see how this could ever have worked since current_picks isn't defined (as far as I can tell) in the necessary scope: https://github.com/hadley/ffscrapr/blob/dev-purrr/R/sleeper_draftpicks.R#L36

Error (/Users/hadleywickham/Desktop/ffscrapr/R/mfl_standings.R:20): ff_standings returns a tibble for each platform currently programmed
Error: GET https://api.myfantasyleague.com/2019/export?TYPE=leagueStandings&L=37920&ALL=1&JSON=1 (mfl/2019/export-89ac14.json)

I guess this should be mocked, but is not?

tanho63 commented 1 year ago

Oops. This has been on my neglected list for a whiiile, thank you for the fixes! I know about the draftpicks issue.

@hadley when did you plan on having purrr submitted to CRAN?

hadley commented 1 year ago

I've also included a partial fix for dev purrr — but I can't quite figure out what the code is doing, so you'll need to figure it out yourself. The root cause is a bug in CRAN purrr where map_depth() would happily map if an object at that depth didn't exist, even though it was supposed to error.

hadley commented 1 year ago

The purrr release isn't currently scheduled, but I'm likely to aim for mid-December.

tanho63 commented 1 year ago

@hadley trying to work on the vec_depth issue and noticed I'm also getting test failures related to tidyverse/purrr#976. Will ignore since issue is already raised (unless you've advice otherwise?)

✖ | 1       0 | ff_transactions [0.6s]                              
────────────────────────────────────────────────────────────────────
Error (test-ff_transactions.R:14:3): ff_transactions returns a tibble of transactions
Error in `map(.x, .f, ...)`: ℹ In index: 1.
Caused by error in `dplyr::mutate()`:
! Problem while computing `franchise_id =
  purrr::map_chr(.data$franchise_id, unlist)`.
Caused by error in `purrr::map_chr()`:
ℹ In index: 1.
Caused by error in `deprecate_to_char()`:
! could not find function "deprecate_to_char"
Backtrace:
  1. ffscrapr::ff_transactions(jml_conn, week = 1:9)
       at test-ff_transactions.R:14:2
 34. dplyr (local) `<fn>`(`<rlng_rrr>`)
`session_info()` ```r R> session_info() ─ Session info ─────────────────────────────────────────────────────────────────────────────────────── setting value version R version 4.2.1 (2022-06-23) os Ubuntu 20.04.5 LTS system x86_64, linux-gnu ui RStudio language en_CA:en collate en_CA.UTF-8 ctype en_CA.UTF-8 tz America/Toronto date 2022-11-01 rstudio 2022.06.0-daily+429 Spotted Wakerobin (desktop) pandoc NA ─ Packages ─────────────────────────────────────────────────────────────────────────────────────────── ! package * version date (UTC) lib source assertthat 0.2.1 2019-03-21 [1] RSPM backports 1.4.1 2021-12-13 [1] RSPM brio 1.1.3 2021-11-30 [1] RSPM cachem 1.0.6 2021-08-19 [1] RSPM callr 3.7.1 2022-07-13 [1] RSPM checkmate * 2.1.0 2022-04-21 [1] RSPM cli 3.4.1 2022-09-23 [1] RSPM crayon 1.5.1 2022-03-26 [1] RSPM curl * 4.3.2 2021-06-23 [1] RSPM data.table 1.14.2 2021-09-27 [1] RSPM DBI 1.1.2 2021-12-20 [1] RSPM desc 1.4.1 2022-03-06 [1] RSPM devtools * 2.4.3 2021-11-30 [1] RSPM digest 0.6.29 2021-12-01 [1] RSPM dplyr 1.0.9 2022-04-28 [1] RSPM ellipsis 0.3.2 2021-04-29 [1] RSPM fansi 1.0.3 2022-03-24 [1] RSPM fastmap 1.1.0 2021-01-25 [1] RSPM VP ffscrapr * 1.4.7.11 2022-05-20 [?] Github (ffverse/ffscrapr@2975df8) (on disk 1.4.7.4) fs 1.5.2 2021-12-08 [1] RSPM generics 0.1.3 2022-07-05 [1] RSPM glue 1.6.2 2022-02-24 [1] RSPM httptest 4.1.0 2021-09-22 [1] RSPM httr 1.4.3 2022-05-04 [1] RSPM jsonlite 1.8.0 2022-02-22 [1] RSPM lifecycle 1.0.3 2022-10-07 [1] RSPM magrittr 2.0.3 2022-03-30 [1] RSPM memoise 2.0.1 2021-11-26 [1] RSPM nflreadr 1.3.0.02 2022-08-08 [1] Github (nflverse/nflreadr@c26142b) pak 0.3.0 2022-04-12 [1] local pillar 1.8.0 2022-07-18 [1] RSPM pkgbuild 1.3.1 2021-12-20 [1] RSPM pkgconfig 2.0.3 2019-09-22 [1] RSPM pkgload 1.3.0 2022-06-27 [1] RSPM prettyunits 1.1.1 2020-01-24 [1] RSPM processx 3.7.0 2022-07-07 [1] RSPM progressr 0.10.1 2022-06-03 [1] RSPM ps 1.7.1 2022-06-18 [1] RSPM purrr 0.9000.0.9000 2022-11-01 [1] Github (tidyverse/purrr@2ad46d5) R6 2.5.1 2021-08-19 [1] RSPM rappdirs 0.3.3 2021-01-31 [1] RSPM ratelimitr 0.4.1 2018-10-07 [1] RSPM remotes 2.4.2 2021-11-30 [1] RSPM rlang 1.0.6 2022-09-24 [1] RSPM rprojroot 2.0.3 2022-04-02 [1] RSPM rstudioapi 0.13 2020-11-12 [1] RSPM sessioninfo 1.2.2 2021-12-06 [1] RSPM stringi 1.7.8 2022-07-11 [1] RSPM stringr 1.4.0 2019-02-10 [1] RSPM testthat * 3.1.4 2022-04-26 [1] RSPM tibble 3.1.8 2022-07-22 [1] RSPM tidyr 1.2.0 2022-02-01 [1] RSPM tidyselect 1.1.2 2022-02-21 [1] RSPM usethis * 2.1.5.9000 2022-05-25 [1] Github (r-lib/usethis@af60dfe) utf8 1.2.2 2021-07-24 [1] RSPM vctrs 0.5.0 2022-10-22 [1] RSPM waldo 0.4.0 2022-03-16 [1] RSPM withr 2.5.0 2022-03-03 [1] RSPM [1] /home/tan/R/x86_64-pc-linux-gnu-library/4.2 [2] /usr/local/lib/R/site-library [3] /usr/lib/R/site-library [4] /usr/lib/R/library V ── Loaded and on-disk version mismatch. P ── Loaded and on-disk path mismatch. ────────────────────────────────────────────────────────────────────────────────────────────────────── ```

edit: nvm I see this will be resolved by tidyverse/purrr#989

hadley commented 1 year ago

While you wait for a fix to trigger the proper deprecation message, you can call as.character() to fix the underlying problem.

tanho63 commented 1 year ago

Looks like this vec_depth() issue is a holdover from when I was trying to unnest a ragged list-column (#344). Seems like purrr::map(dplyr::bind_rows) will work without the ragged extraction, so I think it does the trick.

tanho63 commented 1 year ago

Thoughts on your other changes (in case you were curious):

I now cache the httptest shims for 24 hours to avoid downloading them every time the tests are run. This probably isn't quite right (I don't know why they are in a separate repo to begin with) and will need a bit more work for CRAN (since I think you'll need to clean up the cache directory in R CMD check)

Mocked API data was previously bundled inside of the package tests, but was quite large and caused the built package to grow larger than 10MB. I didn't know about the existence of piggyback at the time or I would have used that and attached the test data to a GH release of this repository. I still might switch this over a bit later.

I replaced with_mock_api() with local_mock_api() so the test back traces are simpler (and clicking on them in RStudio takes you to the right place) I switched from setup.R to helper-mocking.R so it all works when you use load_all()

These all look good to me. I added back a check to see if GitHub was online, because at one point I got BDR'ed for GitHub being offline when it was trying to download mock data and GH was not online 😦

Thank you for taking the time to work on this, and for making purrr and the tidyverse awesome!

hadley commented 1 year ago

Why not just leave the test data in the same repo, but .Rbuildignored? IMO you really shouldn't be downloading these files during CRAN tests.

tanho63 commented 1 year ago

I think that would require skip_on_cran, which would have prevented the purrr revdepcheck from identifying this problem altogether - and I wanted to be robust against dependency changes

hadley commented 1 year ago

Avoiding the package size limitation by downloading a file during testing feels a bit dubious to me. If CRAN hasn't complained specifically about it, you're probably ok, but I'd be prepared to rework it if they do notice and don't like it.

hadley commented 1 year ago

FYI purrr is now scheduled for release to CRAN on Dec 19