Closed GoogleCodeExporter closed 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
I'll try right now!)
Original comment by k...@k-bx.com
on 4 Apr 2012 at 10:25
Done.
Original comment by k...@k-bx.com
on 4 Apr 2012 at 11:16
Attachments:
this one is better, sorry
Original comment by k...@k-bx.com
on 4 Apr 2012 at 11:18
Attachments:
[deleted comment]
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
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
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
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
Original comment by fuzzyman
on 4 Apr 2012 at 1:44
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:
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
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
What name should I use to credit you in the docs for this?
Original comment by fuzzyman
on 13 Apr 2012 at 4:10
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
Konstantine Rybnikov. Thank you.
Original comment by k...@k-bx.com
on 14 Apr 2012 at 9:07
Original issue reported on code.google.com by
k...@k-bx.com
on 4 Apr 2012 at 9:26Attachments: