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

fix #76 by fully qualifying/scoping the httptest function calls in th… #77

Merged
merged 1 commit into from
Nov 16, 2022
Merged

fix #76 by fully qualifying/scoping the httptest function calls in th… #77

merged 1 commit into from
Nov 16, 2022

Conversation

kforner
Copy link
Contributor

@kforner kforner commented Nov 15, 2022

fix #76 by fully qualifying/scoping the httptest function calls in the instrumented code.

Tested locally with my use case: I could use httptest in Suggests: instead of in Depends: in my package and it still works as expected.
And the tests still pass.

Copy link
Owner

@nealrichardson nealrichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Looks like the same issue would occur in httptest2: https://github.com/nealrichardson/httptest2/blob/main/R/capture-requests.R

Would you be willing to make a similar PR over there as well?

@nealrichardson nealrichardson merged commit 77a2acb into nealrichardson:master Nov 16, 2022
@kforner
Copy link
Contributor Author

kforner commented Nov 17, 2022

Sure. Quick question: Why aren't you using httr builtin callback feature to capture the requests (set_callback()) ?

@nealrichardson
Copy link
Owner

I don't recall for sure at this point, but the original implementation of httptest predates that feature in httr, so historically that's why it relies on trace(). There may also be a difference in how request errors are handled--I feel like I tried to switch to use the callbacks at some point and ran into an issue.

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.

start_capturing() should use fully scoped function calls
2 participants