Merge lp:~elopio/payclient/fix1165210-mock_factory into lp:payclient

[Leo Arias]

Use a mock for the test_factory_from_data_objects.

[lp:~elopio/payclient/fix1165210-mock_factory]

6. By Leo Arias on 2013-04-08

Moved the mock name to a variable.

[Leo Arias (elopio)]

<pindonga> elopio, don't like the \ on l. 11
<pindonga> can you replace that for
<pindonga> name = 'payclient.client.CreditCardRequest.__init__'
<pindonga> with patch(name) as mock_request:
<elopio> pindonga: I can, sure.
<pindonga> rest looks good
<elopio> pindonga: I would call it something like mock_name, though
<elopio> pindonga: and what I'm not so sure about is that mfoord said it's generally not a good idea to mock the __init__
<elopio> but I found no other way here.
<pindonga> what about mocking out the class completely?
<elopio> pindonga: that won't work, because I need to call .from_data_objects
<pindonga> k
<elopio> maybe I can ask him tomorrow to give it a look before landing.
<pindonga> sure
<pindonga> it looks good to me
<pindonga> I can comment approve and leave it up to you to do the final bit
<elopio> thanks pindonga. My mistake on friday was that I missed the return_value. It was dumb.
<pindonga> np

[Ricardo Kirkner (ricardokirkner)]

LGTM

review: Approve

[Leo Arias (elopio)]

<mfoord> elopio: I don't think you actually need to do it
<mfoord> import CreditCardRequest first (in the normal way at the module level) so you have a reference to it
<elopio> mfoord: interesting. is there a better way?
<mfoord> then call CreditCardRequest.from_data_objects
<mfoord> but with the patch in place - in the form patch(mock_name)
<mfoord> from_data_objects (presumably) instantiates CreditCardRequest
<mfoord> it will look up the *name* CreditCardRequest inside its own namespace - and find the overridden name (i.e. a mock)
<mfoord> try it and see if it works
<mfoord> it should do
<elopio> ok, let me try to parse it. I'll try, thanks mfoord.
<mfoord> hehe
<mfoord> at the top of the module:
<mfoord> from payclient.client import CreditCardRequest
<mfoord> then remove the .__init__ from mock_name
<mfoord> and instead of payclient.client.CreditCardRequest.from_data_objects(...)
<mfoord> you should be able to call
<mfoord> CreditCardRequest.from_data_objects(...)
<mfoord> and the rest should work unchanged
<elopio> mfoord: got it. Sounds good.
<elopio> mfoord: it fails: http://paste.ubuntu.com/5692839/
<mfoord> interesting
<mfoord> can you pastebin the code for from_data_objects ?
<elopio> http://paste.ubuntu.com/5692854/
<elopio> mfoord: ^^
<mfoord> gah, dammit
<mfoord> it uses cls
<mfoord> so it doesn't use a name lookup
<mfoord> elopio: back to your old approach, sorry for the blind alley
<elopio> mfoord: don't worry, I've learned plenty of things with this branch :)
<mfoord> haha, sorry
<elopio> my first try was really lame :) I still need a real world escenario for the magicmock to fully understand it.
<elopio> mfoord: so, can I land it as is?
<mfoord> yep

=== modified file 'src/payclient/tests/test_client.py'
--- src/payclient/tests/test_client.py	2013-04-05 21:08:07 +0000
+++ src/payclient/tests/test_client.py	2013-04-08 18:00:30 +0000
@@ -111,9 +111,12 @@
         billing_address = payclient.data.Address(
             street, state, postal_code, country_code)
 
-        request = CreditCardRequest.from_data_objects(
-            shop_id, user_open_id, credit_card, billing_address,
-            make_unattended_default)
+        mock_name = 'payclient.client.CreditCardRequest.__init__'
+        with patch(mock_name) as mock_request:
+            mock_request.return_value=None
+            payclient.client.CreditCardRequest.from_data_objects(
+                shop_id, user_open_id, credit_card, billing_address,
+                make_unattended_default)
 
         billing_address_dict = {
             'street': billing_address.street,
@@ -121,18 +124,16 @@
             'country': billing_address.country_code,
             'postal': billing_address.postal_code
         }
-        expected_attributes = (
-            shop_id, user_open_id, card_type, name_on_card, card_number,
-            ccv_number, expiration_month, expiration_year,
-            make_unattended_default, billing_address_dict
-        )
-        actual_attributes = (
-            request.shop_id, request.open_id, request.card_type,
-            request.card_holder, request.card_number, request.card_ccv,
-            request.card_expiration_month, request.card_expiration_year,
-            request.make_unattended_default, request.billing_address
-        )
-        self.assertEquals(expected_attributes, actual_attributes)
+        mock_request.assert_called_with(
+            shop_id=shop_id, open_id=user_open_id,
+            card_type=credit_card.card_type, 
+            card_holder=credit_card.name_on_card,
+            card_number=credit_card.card_number,
+            card_ccv=credit_card.ccv_number,
+            card_expiration_month=credit_card.expiration_month,
+            card_expiration_year=credit_card.expiration_year,
+            billing_address=billing_address_dict,
+            make_unattended_default=make_unattended_default)
 
 
 class PaymentRequestTestCase(TestCase):