-
Notifications
You must be signed in to change notification settings - Fork 31
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
unit tests should exercise all combinations of Net::Stripe object argument types #139
Comments
correction, the combinations for that is, unless we are allowing for cases where the user calls so that leaves us with more like 15 necessary tests in the case of |
also worth noting, for optional arguments, there are cases that are not as clear cut as "are we creating or updating a customer?" for example, where feasible, i am inclined to generate explicit errors in |
* updated Kavorka signature to remove non-functional or illegitimate argument types * removed Net::Stripe::Card and disallowed card id for card, as neither form is valid conceptually <lukec#138> * always create a Net::Stripe::Customer object before _post() to take advantage of argument coercion during objectification <lukec#148> * include omitted arguments in object creation * clean up and centralize Net::Stripe:Token coercion code, since we always need the token id * added unit tests to exercise all allowed argument forms for customer creation and customer update <lukec#139> * closes <lukec#138>
* updated Kavorka signature to remove non-functional or illegitimate argument types * removed Net::Stripe::Card and disallowed card id for card, as neither form is valid conceptually <#138> * always create a Net::Stripe::Customer object before _post() to take advantage of argument coercion during objectification <#148> * include omitted arguments in object creation * clean up and centralize Net::Stripe:Token coercion code, since we always need the token id * added unit tests to exercise all allowed argument forms for customer creation and customer update <#139> * closes <#138>
this is a general note, and a very-long-term todo list item.
basically, what i have discovered while working on #94, #134, #137, #138, etc, is that there are various combinations of argument types that we are not testing for, based on the values we claim to accept in the Kavorka signature for a given method.
for example, take
post_customer()
. it acceptsNet::Stripe::Customer|Str
forcustomer
and it acceptsNet::Stripe::Card|Net::Stripe::Token|Str|HashRef
forcard
withStr
representing either token id or card id. that leaves us with 2 x 5 combinations that we should be testing.then, add to that the fact that
post_customer()
can be called to create a customer or update an existing customer, and we have 2 x 2 x 5 combinations.while this might seem like overkill, the reality is that any of these individual combinations is subject to fail either because of how the method is written or because of the restrictions of the Stripe API. for example, the API call for
post_charge()
works when passing a token id with no customer, but fails when passing token id with an existing customer.so my current approach, which is subject to change :-), is to add appropriate unit tests for the stated
Net::Stripe
object arguments, and to either add more-nuanced validation to provide a predictable, pre-API-call error for the combinations that we know not to work, or to remove the TypeConstraint from the Kavorka signature if an argument type fails in all combinations.side note: the parameter validation seems to be made a little bit cleaner by having separate methods for creation and updating in cases where the acceptable argument types differ between the two calling forms.
The text was updated successfully, but these errors were encountered: