search

derwiki-adroll / mock

Automatically exported from code.google.com/p/mock
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Skip AttributeError on autospec=True #152

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Currently, when you use django (djangotoolbox in my case), dir() on model 
description will give you fields, but when you try to do getattr() on those 
fields, you will get AttributeError.

I propose to skip AttributeErrors like this. What do you think?

======================================================================
ERROR: test_should_put_recognized_products_to_receipt 
(shwp.consumer.tests.bl.ocr_test.TestPutSucceededOcrResultIntoReceipt)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kost/.virtualenvs/shp/local/lib/python2.7/site-packages/mock-0.9.0alpha1-py2.7.egg/mock.py", line 1216, in patched
    arg = patching.__enter__()
  File "/home/kost/.virtualenvs/shp/local/lib/python2.7/site-packages/mock-0.9.0alpha1-py2.7.egg/mock.py", line 1336, in __enter__
    _name=self.attribute, **kwargs)
  File "/home/kost/.virtualenvs/shp/local/lib/python2.7/site-packages/mock-0.9.0alpha1-py2.7.egg/mock.py", line 2178, in create_autospec
    _name='()', _parent=mock)
  File "/home/kost/.virtualenvs/shp/local/lib/python2.7/site-packages/mock-0.9.0alpha1-py2.7.egg/mock.py", line 2197, in create_autospec
    original = getattr(spec, entry)
  File "/home/kost/.virtualenvs/shp/local/lib/python2.7/site-packages/djangotoolbox/fields.py", line 21, in __get__
    raise AttributeError('Can only be accessed via an instance.')
AttributeError: Can only be accessed via an instance.

Original issue reported on code.google.com by k...@k-bx.com on 4 Apr 2012 at 9:26

Attachments:

GoogleCodeExporter commented 9 years ago 
Yep, a good point. Care to write a test for the change? (Noting that the 
created mock should not have this attribute and should also raise an attribute 
error - I *think* you get this for free but it needs to be checked.)

Original comment by fuzzyman on 4 Apr 2012 at 10:15

GoogleCodeExporter commented 9 years ago 
I'll try right now!)

Original comment by k...@k-bx.com on 4 Apr 2012 at 10:25

GoogleCodeExporter commented 9 years ago 
Done.

Original comment by k...@k-bx.com on 4 Apr 2012 at 11:16

Attachments:

GoogleCodeExporter commented 9 years ago 
this one is better, sorry

Original comment by k...@k-bx.com on 4 Apr 2012 at 11:18

Attachments:

GoogleCodeExporter commented 9 years ago 
[deleted comment]
GoogleCodeExporter commented 9 years ago 
Your test doesn't check that accessing s.raiser causes an AttributeError - or 
is your intention that something in dir(obj) that can't be fetched from the 
original should still be accessible on the mock created by autospec? If that is 
the case what would be the spec for s.raiser? 

If there is no spec, so all attributes should be accessible, then the test 
should check that.

Original comment by fuzzyman on 4 Apr 2012 at 12:11

GoogleCodeExporter commented 9 years ago 
My intention is that if something is accessible on dir() but non-accessible on 
getattr(), it should be skipped (since you cannot build spec of something you 
cannot access).

But if you think that if something from dir() on getattr() throws exception we 
should copy that behaviour -- I wouldn't mind (I can even implement it). I just 
have a feeling that it can restrict us somewhere else.

So should we do something like:
except Exception, e:
    m = MagicMock()
    m.side_effect = lambda: throw e
    setattr(obj, field_name, m)

?

Original comment by k...@k-bx.com on 4 Apr 2012 at 1:31

GoogleCodeExporter commented 9 years ago 
I mean, what I would do -- I'd rather add a line "s.raiser" that would check 
that it doesn't raise anything (if you need, I can modify patch for that).

Original comment by k...@k-bx.com on 4 Apr 2012 at 1:32

GoogleCodeExporter commented 9 years ago 
Right, I'm not sure what you mean by "skipped" (and whatever you mean should be 
tested). :-) I think you mean it should be accessible but have no spec, in 
which case test that accessing it doesn't raise an attribute error and that you 
can access arbitrary attributes on it.

Original comment by fuzzyman on 4 Apr 2012 at 1:33

GoogleCodeExporter commented 9 years ago

Original comment by fuzzyman on 4 Apr 2012 at 1:44

GoogleCodeExporter commented 9 years ago 
By skipped, I mean "continue" from my original patch with implementation. Yes, 
I agree, it should be accessible and have no spec, even though original object 
raises exception on getattr(). So here's a full test that would check that it's 
accessible (added s.raiser str).

Original comment by k...@k-bx.com on 4 Apr 2012 at 2:00

Attachments:

GoogleCodeExporter commented 9 years ago 
It is important to specify, and test, the behaviour from the point of view of 
the end user. The fact that the *implementation* uses a continue to skip the 
attribute does not specify the expected behaviour for the user of the mock. I 
think we got there though - thanks very much, I think this is a good 
improvement to autospec.

Original comment by fuzzyman on 4 Apr 2012 at 2:14

GoogleCodeExporter commented 9 years ago 
Yeah, sorry for some unclear things, I now understand what you've meant. Thanks!

Original comment by k...@k-bx.com on 4 Apr 2012 at 3:31

GoogleCodeExporter commented 9 years ago 
What name should I use to credit you in the docs for this?

Original comment by fuzzyman on 13 Apr 2012 at 4:10

GoogleCodeExporter commented 9 years ago 
This is now applied on trunk and will be in 1.0. Many thanks.

Original comment by fuzzyman on 13 Apr 2012 at 7:56

GoogleCodeExporter commented 9 years ago 
Konstantine Rybnikov. Thank you.

Original comment by k...@k-bx.com on 14 Apr 2012 at 9:07