-
Notifications
You must be signed in to change notification settings - Fork 12
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
Problem with filter_sensitive_data
not replacing keys
#167
Comments
thanks for this @juliasilge is there any chance I can try to replicate the issue? I don't have a qualtrics account, but is there a sandbox qualtrics instance I can use? |
The free accounts unfortunately don't allow API access. I will try to see if there is some way to share info. |
it looks like in all of the tests in that PR that the if i comment out the line calling |
or keep the |
@juliasilge did that help? |
Thank you so much for looking at this! I didn't clarify before that I was removing that line with the test_that("all_mailinglists returns a tbl_df with expected column names and types", {
readRenviron("~/.Renviron")
vcr::use_cassette("all_mailinglists", {
x <- all_mailinglists()
})
expect_s3_class(x, c("tbl_df","tbl","data.frame"))
expect_named(x, c("libraryId", "id", "name", "category", "folder"))
expect_type(x$libraryId, "character")
expect_type(x$id, "character")
expect_type(x$name, "character")
expect_type(x$category, "character")
expect_type(x$folder, "character")
})
Then a fixture Another contributor is trying to just use webmockr instead and we are having trouble with that too. 😬 This is at least more reproducible so I will open an issue over there. |
Thanks for the update. Do you have other instances of |
OK, it took some doing but I think we got vcr up and running for qualtRics. I did have to do some manual editing of the fixture
Feel free to close this issue if you like. It seems like the main underlying problem for us was webmockr and registered/unregistered requests? I think that vcr is behaving as expected although I am not 100% sure. |
Thanks Julia. Hmm, that workflow shouldn't be necessary. I'm not quite sure what's going on - you shouldn't have to manually edit the fixtures, so we have more work to do here to make it so that you just wrap your calls in
i see that you have some test files that use webmockr and some that use vcr. Are you saying above that you get webmockr errors for the tests that use vcr? |
Yes, that's right -- webmockr errors (unregistered requests) on the vcr tests. |
Weird. I don't think that should be happening. I'm having a hard time replicating your errors.. It seems you still have those calls to |
I remove those lines when I want to use real API credentials that are in my The tests succeed now because the fake values are loaded into the environment variables, and the fake API calls are loaded via vcr. |
Okay, thanks for the explanation. Sorry about this not working super cleanly. May be related to #137 |
We actually closed #137 by adding docs, and reading this issue I am not sure it was related but I might be missing something. In (small) packages where I have done vcr stuff with secrets
if(!nzchar(Sys.setenv("APIKET")) {
Sys.setenv("APIKEY" = "foobar")
} in order to fool the package (not vcr). I'll go read the PR thread in qualtRics. |
thxs @maelle |
In the end I wasn't able to find out what the vcr/webmockr problem was here. |
ugh, well thanks for trying |
Hello! 👋 We are trying to get vcr tests set up in the qualtRics package and are running into some challenges. The first one is that
filter_sensitive_data
does not seem to actually be filtering out the important strings. This work is being done in ropensci/qualtRics#140 so you can check out what's in the currenthelper-qualtRics.R
file if you like:When I run
devtools::test()
I do have new fixtures being created for the two functions we are currently trying to test using vcr, but they have my API key and personal URL in them recorded on disk. They must be in the environment at the time that the API call is being made or otherwise the call would fail.Do you have any ideas? Have you seen other problems with the sensitive data issue?
The text was updated successfully, but these errors were encountered: