-
Notifications
You must be signed in to change notification settings - Fork 37
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] Add library hooks #277
Conversation
public static Object testRequestHooks(RequestHookResponses datas) { | ||
assertEquals("https://api.easypost.com/v2/parcels", datas.getPath()); | ||
assertEquals("POST", datas.getMethod()); | ||
assertNotNull(datas.getHeaders()); | ||
assertNotNull(datas.getRequestBody()); | ||
assertNotNull(datas.getRequestTimestamp()); | ||
assertNotNull(datas.getRequestUuid()); | ||
|
||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see a follow-up PR where we rename datas
to data
. People will use tests as an example and datas
plural doesn't make sense.
Same with RequestHookResponses
should not be plural and be RequestHookResponse
instead
*/ | ||
@Test | ||
public void testCreateParcelWithRequestHook() throws EasyPostException { | ||
vcr.setUpTest("create"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like if we renamed the create
to something different since we aren't really creating a hook but are instead capturing the details of a request that we hooked into. Renaming the cassette will help reduce confusion about its purpose.
Description
RequestHook
andResponseHook
classes. (un)subscribe to them with the newsubscribeToRequestHook
,subscribeToResponseHook
,unsubscribeFromRequestHook
, orunsubscribeFromResponseHook
methods of anEasyPostClient
Testing
Add new unit tests
Pull Request Type
Please select the option(s) that are relevant to this PR.