Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Womens testthat tests failing in travis CI #53

Closed
jimmyday12 opened this issue Dec 11, 2018 · 6 comments
Closed

Womens testthat tests failing in travis CI #53

jimmyday12 opened this issue Dec 11, 2018 · 6 comments
Labels
bug

Comments

@jimmyday12
Copy link
Owner

@jimmyday12 jimmyday12 commented Dec 11, 2018

Likely related to #52 - the womens tests are failing. Again - can't replicate locally, only fails on Travis.

  ══ testthat results  ═══════════════════════════════════════════════════════════
  OK: 66 SKIPPED: 0 FAILED: 2
  1. Error: get_aflw_round_data returns data frame with correct variables (@test-womens_stats.R#18) 
  2. Error: get_aflw_match_data returns dataframe with correct variables (@test-womens_stats.R#38) 
  
  Error: testthat unit tests failed

@jimmyday12
Copy link
Owner Author

@jimmyday12 jimmyday12 commented Dec 11, 2018

@OscarLane Just tagging you here again - I suspect this is what is causing the vignette to fail in #52.

I tried testing locally and when I run devtools::test(), for some reason the code enters debug mode.

debug at /Users/jamesday/Documents/R_scripts/fitzRoy/R/womens_stats.R#236: match_data <- request_metadata %>% jsonlite::fromJSON(flatten = TRUE) %>% 
    .$lists %>% dplyr::as_data_frame() %>% dplyr::mutate(Match.Id = matchid, 
    Round.Id = roundid, Competition.Id = competitionid) %>% dplyr::mutate(home.away = c("Home", 
    "Away")) %>% tidyr::gather(stat, value, .data$stats.averages.goals:.data$team.teamNickname) %>% 
    dplyr::mutate(stat = case_when(.data$home.away == "Home" ~ 
        stringr::str_c("home.", .data$stat), .data$home.away == 
        "Away" ~ stringr::str_c("away.", .data$stat))) %>% dplyr::select(-.data$home.away) %>% 
    tidyr::spread(.data$stat, .data$value) %>% dplyr::rename_at(dplyr::vars(contains(".lastUpdated")), 
    funs(stringr::str_replace(., "stats", "STATS"))) %>% dplyr::mutate_at(dplyr::vars(tidyselect::contains(".stats", 
    ignore.case = FALSE)), readr::parse_number) %>% dplyr::rename_at(dplyr::vars(tidyselect::contains(".lastUpdated")), 
    funs(stringr::str_replace(., "STATS", "stats"))) %>% dplyr::mutate_at(dplyr::vars(tidyselect::contains(".lastUpdated")), 
    readr::parse_datetime)

I'm unsure what this means to enter debug mode!

I tried running the example listed in the documentation and get the same issue. I changed the code a bit and managed to isolate the issue to two lines of code;

request_metadata %>% jsonlite::fromJSON(flatten = TRUE and .$lists.

get_aflw_detailed_match_data("CD_M20172640101","CD_R201726401", "CD_S2017264", get_aflw_cookie() )
Called from: eval(expr, p)
Browse[1]> n
debug at /Users/jamesday/Documents/R_scripts/fitzRoy/R/womens_stats.R#236: match_data <- request_metadata %>% jsonlite::fromJSON(flatten = TRUE)
Browse[2]> n
debug at /Users/jamesday/Documents/R_scripts/fitzRoy/R/womens_stats.R#239: match_data <- match_data %>% .$lists
Browse[2]> 

It seems that for some reason, both of these lines trigger a debug mode in R Studio (which is potentially related to the R CMD CHECK failing). I don't even know how that's possible but I'm thinking there may be some strange text in the actual parsed data causing this? Not sure if you can replicate on your machine or have any ideas @OscarLane?

@jimmyday12
Copy link
Owner Author

@jimmyday12 jimmyday12 commented Dec 11, 2018

Further to this, running the code in the console rather than in the get_aflw_detailed_match_data function doesn't produce the strange debug error.

library(dplyr)
matchid = "CD_M20172640101"
competitionid = "CD_S2017264"
roundid = "CD_R201726401"
cookie = fitzRoy::get_aflw_cookie()

request_metadata <- httr::GET("http://www.afl.com.au/api/cfs/afl/statsCentre/teams",
                              query = list(matchId = matchid,
                                           roundId = roundid,
                                           competitionId = competitionid),
                              httr::add_headers(`X-media-mis-token` = cookie)) %>% 
  httr::content(as = "text", encoding = "UTF-8")

# Check that round info is available on web, if not return error
if (stringr::str_detect(request_metadata, "Page Not Found")) {
  stop(paste0("Invalid match ID (", matchid, "). Have you checked that this ",
              "game has been played yet?"))
}
match_data <- request_metadata %>% 
  jsonlite::fromJSON(flatten = T) 

match_data <- match_data %>% 
  .$lists 

glimpse(match_data[1:10])
#> Observations: 2
#> Variables: 10
#> $ stats.averages.goals                <dbl> 5.6, 4.6
#> $ stats.averages.behinds              <dbl> 2.7, 4.1
#> $ stats.averages.superGoals           <lgl> NA, NA
#> $ stats.averages.kicks                <dbl> 111.4, 119.7
#> $ stats.averages.handballs            <dbl> 60.6, 47.9
#> $ stats.averages.disposals            <dbl> 172.0, 167.6
#> $ stats.averages.marks                <dbl> 32.7, 39.7
#> $ stats.averages.bounces              <dbl> 2.1, 3.7
#> $ stats.averages.tackles              <dbl> 49.4, 56.6
#> $ stats.averages.contestedPossessions <dbl> 96.1, 90.3

Created on 2018-12-11 by the reprex package (v0.2.1)

@jimmyday12 jimmyday12 added the bug label Dec 11, 2018
@jimmyday12
Copy link
Owner Author

@jimmyday12 jimmyday12 commented Dec 12, 2018

Hmm so that random debug error fixed itself with a restart of my laptap - must have been a strange breakpoint somewhere that I couldn't find.

Nonetheless, the Travis CI build is still failing and I can't replicate locally. Seems to fail on get_aflw_match_data() in the examples as well as on that same function in the unit tests.

checking examples ... ERROR
Running examples in ‘fitzRoy-Ex.R’ failed
The error most likely occurred in:
> ### Name: get_aflw_match_data
> ### Title: Get AFLW match data
> ### Aliases: get_aflw_match_data
> 
> ### ** Examples
> 
> ## All data
> get_aflw_match_data()
Error in mutate_impl(.data, dots) : 
  Evaluation error: is.character(x) is not TRUE.
Calls: get_aflw_match_data ... <Anonymous> -> mutate.tbl_df -> mutate_impl -> .Call
Execution haltedchecking tests ... ERROR
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  15: withVisible(function_list[[k]](value))
  16: function_list[[k]](value)
  17: dplyr::mutate(., Local.Start.Time = readr::parse_datetime(Local.Start.Time))
  18: mutate.tbl_df(., Local.Start.Time = readr::parse_datetime(Local.Start.Time))
  19: mutate_impl(.data, dots)
  
  ══ testthat results  ═══════════════════════════════════════════════════════════
  OK: 66 SKIPPED: 0 FAILED: 2
  1. Error: get_aflw_round_data returns data frame with correct variables (@test-womens_stats.R#18) 
  2. Error: get_aflw_match_data returns dataframe with correct variables (@test-womens_stats.R#38) 
@OscarLane
Copy link
Contributor

@OscarLane OscarLane commented Dec 12, 2018

Hmm interesting, I'll try to have a look this weekend. I'm not getting any problems running the function, so it looks like it's a problem with the test. Possibly the underlying columns available have changed, meaning the test no longer passes, as it looks for exact matches.

@OscarLane
Copy link
Contributor

@OscarLane OscarLane commented Jan 2, 2019

I had a look at this finally (pesky holidays getting in the way!), and am as stumped as you are. I've not used Travis before, but I guess it could be something to do with the Travis configuration? Do you think it could be something to do with building in an older version of R? (I see .travis.yaml lists 3.2, 3.3, 3.4 and release.)

@jimmyday12
Copy link
Owner Author

@jimmyday12 jimmyday12 commented Jan 11, 2019

Seemed to have fixed this in the commit a8902ce. Was related to a line of code in get_aflw_round_data() that was using readr::parse_datetime. I've commented that line out and it seems to work. Also has fixes #52!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.