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

feature/webmockr #161

Merged
merged 14 commits into from
May 7, 2020
Merged

feature/webmockr #161

merged 14 commits into from
May 7, 2020

Conversation

shaun-jacks
Copy link
Contributor

@shaun-jacks shaun-jacks commented May 5, 2020

Hello @juliasilge,
This PR replaces httptest with webmockr. I added unit tests to check http requests on Qualtrics APIs for various functions in the package. And also added other misc. tests.

I went with webmockr because mocking the http requests and testing the http request structure felt like it made the most sense for this package. It could be difficult authenticating with Qualtrics and caching actual requests during CI, and I don't think it's necessary to do this / I am not really sure how to go about that to be honest. This PR also has tests for additional qualtRics functions such as qualtrics_api_request, metadata, and download_qualtrics_export, where the http request structure is tested via stubbing the expected request.

I did my best not to modify any of the library code, and only add tests code, to ensure backwards compatibility. I only removed the package code that was related to httptest within download_qualtrics_export.

It was a little challenging structuring the tests for these functions, so a future idea could be to refactor some of the code to make it more test friendly! I had to mock specific functions in order to get the tests to both run and test specific things.

I have more experience with tests in other languages, but not much experience within R, so any feedback would be much appreciated :) One thing is there is a lot of setup code per test, wish R had a beforeEach type of function... I hope this helps at least provide a base to integrating webmockr and making the testing more robust! Also would like to know if there are any other library or continuous integration modifications needed to integrate this?

@juliasilge
Copy link
Collaborator

This looks extremely promising, @shaun-jacks! Thank you so much for working on this. 🙌

I am trying to figure out what is going on with this mocking to see why the tests are not passing.

First, did the tests pass for you locally?

Second, check out what I am seeing locally:

library(qualtRics)
library(webmockr)

webmockr::enable()
#> CrulAdapter enabled!
#> HttrAdapter enabled!
expected <- list(
  verb     = "GET",
  base_url = "test.qualtrics.com",
  url      = "test.qualtrics.com/API/v3/surveys/",
  api_key  = "1234",
  headers  = list(
    'X-API-TOKEN' = "1234",
    'Content-Type' = 'application/json',
    'Accept' = '*/*',
    'accept-encoding' = 'gzip, deflate'
  )
)

qualtrics_api_credentials(expected$api_key, expected$base_url)
#> To install your credentials for use in future sessions, run this function with `install = TRUE`.

stub_request(expected$verb, expected$url) %>%
  wi_th(headers = expected$headers)
#> <webmockr stub> 
#>   method: get
#>   uri: test.qualtrics.com/API/v3/surveys/
#>   with: 
#>     query: 
#>     body: 
#>     request_headers: X-API-TOKEN=1234, Content-Type=application/json, Accept=*/*, accept-encoding=gzi
#>   to_return: 
#>     status: 
#>     body: 
#>     response_headers: 
#>   should_timeout: FALSE
#>   should_raise: FALSE

## the stub is there
stub_registry()
#> <webmockr stub registry> 
#>  Registered Stubs
#>    GET: test.qualtrics.com/API/v3/surveys/   with headers {"X-API-TOKEN":"1234","Content-Type":"application/json","Accept":"*/*","accept-encoding":"gzip, deflate"}

## why is this request unregistered???
all_surveys()
#> Error: Real HTTP connections are disabled.
#> Unregistered request:
#>   GET:  test.qualtrics.com/API/v3/surveys/   with headers {X-API-TOKEN: 1234, Content-Type: application/json, Accept: */*, accept-encoding: gzip, deflate}
#> 
#> You can stub this request with the following snippet:
#> 
#>    stub_request('get', uri = 'test.qualtrics.com/API/v3/surveys/') %>%
#>      wi_th(
#>        headers = list('X-API-TOKEN' = '1234', 'Content-Type' = 'application/json', 'Accept' = '*/*', 'accept-encoding' = 'gzip, deflate')
#>      )
#> 
#> registered request stubs:
#> 
#>  GET: test.qualtrics.com/API/v3/surveys/   with headers {"X-API-TOKEN":"1234","Content-Type":"application/json","Accept":"*/*","accept-encoding":"gzip, deflate"}
#> ============================================================

## I could register it again...
stub_request('get', uri = 'test.qualtrics.com/API/v3/surveys/') %>%
  wi_th(
    headers = list('X-API-TOKEN' = '1234', 'Content-Type' = 'application/json', 'Accept' = '*/*', 'accept-encoding' = 'gzip, deflate')
  )
#> <webmockr stub> 
#>   method: get
#>   uri: test.qualtrics.com/API/v3/surveys/
#>   with: 
#>     query: 
#>     body: 
#>     request_headers: X-API-TOKEN=1234, Content-Type=application/json, Accept=*/*, accept-encoding=gzi
#>   to_return: 
#>     status: 
#>     body: 
#>     response_headers: 
#>   should_timeout: FALSE
#>   should_raise: FALSE

## but then there are 2!
stub_registry()
#> <webmockr stub registry> 
#>  Registered Stubs
#>    GET: test.qualtrics.com/API/v3/surveys/   with headers {"X-API-TOKEN":"1234","Content-Type":"application/json","Accept":"*/*","accept-encoding":"gzip, deflate"} 
#>    GET: test.qualtrics.com/API/v3/surveys/   with headers {"X-API-TOKEN":"1234","Content-Type":"application/json","Accept":"*/*","accept-encoding":"gzip, deflate"}

## still doesn't work :(
all_surveys()
#> Error: Real HTTP connections are disabled.
#> Unregistered request:
#>   GET:  test.qualtrics.com/API/v3/surveys/   with headers {X-API-TOKEN: 1234, Content-Type: application/json, Accept: */*, accept-encoding: gzip, deflate}
#> 
#> You can stub this request with the following snippet:
#> 
#>    stub_request('get', uri = 'test.qualtrics.com/API/v3/surveys/') %>%
#>      wi_th(
#>        headers = list('X-API-TOKEN' = '1234', 'Content-Type' = 'application/json', 'Accept' = '*/*', 'accept-encoding' = 'gzip, deflate')
#>      )
#> 
#> registered request stubs:
#> 
#>  GET: test.qualtrics.com/API/v3/surveys/   with headers {"X-API-TOKEN":"1234","Content-Type":"application/json","Accept":"*/*","accept-encoding":"gzip, deflate"}
#>  GET: test.qualtrics.com/API/v3/surveys/   with headers {"X-API-TOKEN":"1234","Content-Type":"application/json","Accept":"*/*","accept-encoding":"gzip, deflate"}
#> ============================================================

webmockr::disable()
#> CrulAdapter disabled!
#> HttrAdapter disabled!

Created on 2020-05-06 by the reprex package (v0.3.0)

The exact stub that it says it needs appears to be registered but it doesn't seem to find it or use it or something. Did you see anything like this?

@shaun-jacks
Copy link
Contributor Author

Hi @juliasilge, all my tests do pass locally, but yes this has happened to me as well while developing. Normally I would do a webmockr::stub_registry_clear() to clear the registry, and then register the stub given in the warning instead. And call all_surveys() to see if it works. Does that work for you? My workflow has just been to do devtools::load_all() and then devtools::test(), after making changes as well.

@shaun-jacks
Copy link
Contributor Author

@juliasilge I also noticed that this may work in some cases:

stub_request('GET', uri = 'test.qualtrics.com/API/v3/surveys/') %>%
  wi_th(
    headers = list('X-API-TOKEN' = '1234', 'Content-Type' = 'application/json', 'Accept' = '*/*', 'accept-encoding' = 'gzip, deflate')
  )

Notice how the verb is in all caps, I think this happened to me once, and making the verb all CAPS got the stub to work

@juliasilge
Copy link
Collaborator

Hmmmm, I have tried those steps and I have yet to get any of the mocked requests to succeed, caps, no caps, defined as strings or passed in as variables, etc etc etc. You can see that

  • the tests are not passing on Travis or appveyor 😕
  • I opened an issue on the webmockr repo to check out what is going on

Something clearly is not quite right, but I think this PR is on the right track and we should get it moving forward.

@shaun-jacks
Copy link
Contributor Author

Aww I see :/ well here is my sessionInfo anyway, thanks for looking into this! And for opening an issue with webmockr as well.

sessionInfo()

R version 3.6.3 (2020-02-29)
Platform: x86_64-apple-darwin15.6.0 (64-bit)
Running under: macOS Mojave 10.14.6

Matrix products: default
BLAS:   /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/3.6/Resources/lib/libRlapack.dylib

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] webmockr_0.6.2.92 qualtRics_3.1.2   testthat_2.3.2   

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.3        compiler_3.6.3    pillar_1.4.3      forcats_0.5.0    
 [5] prettyunits_1.1.1 remotes_2.1.1     tools_3.6.3       digest_0.6.25    
 [9] pkgbuild_1.0.6    pkgload_1.0.2     lifecycle_0.2.0   jsonlite_1.6.1   
[13] memoise_1.1.0     tibble_2.1.3      pkgconfig_2.0.3   rlang_0.4.5      
[17] cli_2.0.2         rstudioapi_0.11   curl_4.3          crul_0.9.0       
[21] yaml_2.2.1        haven_2.2.0       dplyr_0.8.5       withr_2.1.2      
[25] httr_1.4.1        stringr_1.4.0     fauxpas_0.5.0     desc_1.2.0       
[29] fs_1.3.2          vctrs_0.2.4       devtools_2.2.2    hms_0.5.3        
[33] triebeard_0.3.0   tidyselect_1.0.0  sjlabelled_1.1.3  rprojroot_1.3-2  
[37] httpcode_0.3.0    glue_1.3.2        R6_2.4.1          processx_3.4.2   
[41] fansi_0.4.1       sessioninfo_1.1.1 whisker_0.4       purrr_0.3.3      
[45] callr_3.4.3       readr_1.3.1       magrittr_1.5      urltools_1.7.3   
[49] backports_1.1.5   ps_1.3.2          ellipsis_0.3.0    usethis_1.5.1    
[53] insight_0.8.2     assertthat_0.2.1  mime_0.9          stringi_1.4.6    
[57] crayon_1.3.4 

@shaun-jacks
Copy link
Contributor Author

If there's anymore I could do I'll be happy to help

@juliasilge
Copy link
Collaborator

OK @shaun-jacks this is good to go now thanks to the help I got on using webmockr!!

Would you like to add yourself as a contributor ("ctb") in the DESCRIPTION file before I merge?

@shaun-jacks
Copy link
Contributor Author

Wow I'm really glad we were able to get it working! Completely forgot to add https to the urls! And I would like to, thank you! :)

@shaun-jacks
Copy link
Contributor Author

Done!

@juliasilge
Copy link
Collaborator

Between this and #140 we went from ~50% to ~80% testing coverage today! 💥

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

Successfully merging this pull request may close these issues.

2 participants