stripe-perl
stripe-perl copied to clipboard
unit tests should exercise all combinations of Net::Stripe object argument types
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 accepts Net::Stripe::Customer|Str
for customer
and it accepts Net::Stripe::Card|Net::Stripe::Token|Str|HashRef
for card
with Str
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.
correction, the combinations for post_customer()
are not 2 x 2 x 5 because some combinations are not valid from a logical standpoint, ie Net::Stripe::Customer
and customer id are not a valid values for customer
if you are creating a new customer.
that is, unless we are allowing for cases where the user calls Net::Stripe::Customer->new()
directly. but my reading of the docs and tests does not indicate that this is expected/allowed.
so that leaves us with more like 15 necessary tests in the case of post_customer()
.
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, post_charge()
works without passing a card
if the customer
passed has an attached card. so we should ensure that our testing and in-method validation takes these nuances into account as well.
where feasible, i am inclined to generate explicit errors in Net::Stripe
rather than unnecessarily hitting the API and simply passing back the produced error. but that does not seem reasonable, in this case at least, since it would require us calling get_customer()
or get_cards()
in order to determine whether the customer has any attached cards.