Add a test that we return the correct number of results

[ledell]

This is a bug that could have been found by having a unit test for this. This ticket is generic, though the bug above is for find_groups() specficially.

[GregSutcliffe]

I took a pass at this but I'm struggling to get the tests to use my existing meetup_token so that I can record the response with vcr.

Here's my test:

test_that("find_groups() can correctly get 2 pages", {
  vcr::use_cassette("find_groups_pages_2", {
    meetup_auth(token = '/path/to/my/meetupr/.httr-oauth.token')
    groups <- find_groups(text = "Ansible", radius = 'global')
  })
  expect_equal(nrow(groups), 324)
})

Which should work (it's what you'd do in the console). But the test fails, reporting:

── Error (test-find_groups2.R:5:3): find_groups() can correctly get 2 pages ────
Error: HTTP 400 Bad Request error encountered for: find/groups.

And when I look at the recorded YAML from vcr, I see (edited for brevity):

- request:
    method: get
    uri: https://api.meetup.com/find/groups?offset=0&text=Ansible&topic_id=&fields=&radius=global
  response:
    status:
      status_code: 400
      category: Client error
      reason: Bad Request
      message: 'Client error: (400) Bad Request'
    body:
      encoding: UTF-8
      file: no
      string: '{"errors":[{"code":"authentication_error","message":"Authenticated
        member required"}]}'

What am I doing wrong?

[GregSutcliffe]

To be clear, obviously I'll be removing the call to meetup_auth() once I have the recorded yaml, I just need a way to authenticate :)

Also, I just tried copying my token to the default path (mycheckout/.httr-oauth) and removing the call to meetup_auth() - still fails.

[maelle]

I think you are bitten by

meetupr/tests/testthat/setup.R

Line 1 in c9c5843

if (nzchar(Sys.getenv("MEETUPR_PWD"))) {

(so not vcr)

To contribute and add tests the best thing is to create a token for tests, now I am wondering what would be the best way to make you add one. I'm looking at ropensci-archive/rtweet#512

[maelle]

that said meetup_token() in that setup file should use your token 🤔

[GregSutcliffe]

Hmm, I added some classic oldschool print statements when running local tests and was fairly sure I wasn't hitting that branch, but rather the else branch. And thus, yes, it should read my token. I will dig a bit more, maybe add some debug in meetup_token()...

[maelle]

thank you! there's definitely a bug somewhere in our codebase.

[GregSutcliffe]

@maelle OK, making progress.

So, this is definitely hitting the else {} branch, which sets MEETUPR_TESTING and then it descends into

meetupr/R/meetup_auth.R

Lines 208 to 211 in fb84cec

	meetup_token <- function(verbose = FALSE) {
	if (nzchar(Sys.getenv("MEETUPR_TESTING"))) {
	return(httr::config())
	}

which clearly skips any kind of token loading. So it's no surprise I can't auth.

I commented that out, which brings us to:

meetupr/R/meetup_auth.R

Lines 212 to 213 in fb84cec

	if (!token_available(verbose = verbose)) meetup_auth(verbose = verbose)
	httr::config(token = .state$token)

And here I get stuck. With some print statements I can see it's trying to load a default token from "~/.local/share/meetupr/meetupr-token.rds" which obviously doesn't exist. I changed it to look like this for debugging:

  meetup_auth(token = '~/path/to/my/real/.httr-oauth', verbose = T)
  message(token_available())
  httr::config(token = .state$token)
}

Which claims to load the token - you can see the verbose output of meetup_auth and token_available:

HttrAdapter enabled!
net connect allowed
<Token>
<oauth_endpoint>
 authorize: https://secure.meetup.com/oauth2/authorize
 access:    https://secure.meetup.com/oauth2/access
<oauth_app> meetup
  key:    <redacted>
  secret: <hidden>
<credentials> access_token, refresh_token, token_type, expires_in
---
TRUE
[ FAIL 1 | WARN 0 | SKIP 0 | PASS 0 ]

However, it fails. But differently, now I get a 401 Unauthorized instead of a 400. So, progress I guess? :)

Is there a way to record an interactive session with vcr? I feel that would be way easier, as my token works fine in the console....

[maelle]

Thank you!

Where is your token? The location With some print statements I can see it's trying to load a default token from "~/.local/share/meetupr/meetupr-token.rds" which obviously doesn't exist. is the new default location.

I'll come back to this within the next few days.

[GregSutcliffe]

So I've been doing quite a lot of digging. Part of the problem is the code creating (broken) tokens which are then hidden from me because I use "git status" to see what I've touched in the repo, but .httr-oauth is in the .gitignore, so I don't see them. I've found several that were not working and getting in the way - that didn't help.

However, I'm now fairly sure it's VCR that is breaking me. Here's my reproducer:

# put token in the default place
cp ~/path/to/my/token /home/greg/.local/share/meetupr/meetupr-token.rds

# bypass the testing environment check
--- a/tests/testthat/setup.R
+++ b/tests/testthat/setup.R
 } else {
-  Sys.setenv("MEETUPR_TESTING" = TRUE)
-  token <- meetup_token()
+ # Sys.setenv("MEETUPR_TESTING" = TRUE)
+  token <- meetup_token(TRUE)
 }

# Write a test that doesnt use vcr
test_that("find_groups() can correctly get 2 pages", {
  groups <- find_groups(text = "Ansible", radius = 'global')
  expect_equal(nrow(groups), 323)
})

This passes, which is a relief:

==> Testing R file using 'testthat'

Loading meetupr
A .httr-oauth file exists in current working directory.
When/if needed, the credentials cached in .httr-oauth will be used for this session.
Or run meetup_auth() for explicit authentication and authorization.

══ Testing test-find_groups2.R ═════════════════════════════════════════════════
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 0 ]Auto-refreshing stale OAuth token.
[ FAIL 0 | WARN 0 | SKIP 0 | PASS 1 ] Done!

Test complete

If I now enable VCR:

test_that("find_groups() can correctly get 2 pages", {
  vcr::use_cassette("find_groups_pages_2", {
    groups <- find_groups(text = "Ansible", radius = 'global')
  })
  expect_equal(nrow(groups), 323)
})

I go back to getting a 401. I wondered if it's related to this:

meetupr/tests/testthat/setup.R

Lines 29 to 35 in fb84cec

	invisible(vcr::vcr_configure(
	filter_request_headers = list(Authorization = "not my bearer token"),
	filter_sensitive_data = list(
	"<<<my_refresh_token>>>" = token$credentials$refresh_token %||% "lala"
	),
	dir = "../fixtures"
	))

But I think that's for filtering the record after the call is complete, right? In any case, I'm now stuck - it seems that the call the vcr is the issue, and I'm not clear on how to debug it.

[GregSutcliffe]

OK, more progress - I figured out how to use vcr on the console to do the recording. So, now I can easily record a YAML with something like this:

devtools::load_all()
meetup_auth()
token <- .state$token
# copied from tests/setup.R
library("vcr")
invisible(vcr::vcr_configure(
  filter_request_headers = list(Authorization = "not my bearer token"),
  filter_sensitive_data = list(
    "<<<my_refresh_token>>>" = token$credentials$refresh_token %||% "lala"
  ),
  dir = here::here("tests/fixtures")
))

use_cassette('greg_test',{
  find_groups('Ansible')
})

This correctly runs, and creates a YAML recording, but it looks like its not capturing the second page, so then I get:

An HTTP request has been made that vcr does not know how to handle:
GET https://api.meetup.com/find/groups?page=200&text=Ansible&radius=global&topic_id=&offset=1&fields=?offset=0
vcr is currently using the following cassette:
  - ../fixtures/find_groups_2_pages.yml
    - record_mode: once
    - match_requests_on: method, uri

Note the page=200 and offset=1, this is the second call in the pagination loop. So progress, I can record something, and get the test to read it. But I'm out of time for today, will poke it more tomorrow ;)

[GregSutcliffe]

@maelle I think I got it, but I'm certain I'm making this harder than it should be. I've got working tests, and I've added them to #111

To get there, I had to work out how to get vcr to run in my console in record mode. Then once I had the yaml, the tests were trivial. To do that, I used this script at the console:

# load meetup
devtools::load_all()

# read my token
meetup_auth(token = '~/.local/share/meetupr/meetupr-token.rds')

# This is copied from setup.R so that VCR deletes sensitive data and writes to the fixtures
library("vcr")
token <- .state$token
invisible(vcr::vcr_configure(
  filter_request_headers = list(Authorization = "not my bearer token"),
  filter_sensitive_data = list(
    "<<<my_refresh_token>>>" = token$credentials$refresh_token %||% "lala"
  ),
  dir = here::here("tests/fixtures")
))

# for some reason i have to run at least one API call outside of vcr, else I get 401s
find_groups('Ansible')

# now I can record the transaction
use_cassette('find_groups_2_pages', record = 'all', {
  find_groups('Ansible')
})

With that, I managed to create two passing tests in #111. Is there a better way?

[maelle]

@GregSutcliffe I'm working on a PR where I change the place of the environment variable that as you correctly pointed out was preventing the addition of new test in the absence of MEETUPR_PWD. I am so sorry and also very thankful that you found this out!

[GregSutcliffe]

No problem @maelle I learned a lot about vcr along the way and that's going to be useful at some point. No learning is ever wasted :)