[Python-Dev] How far to go with user-friendliness

[Ron Adam]

On 07/19/2015 02:33 PM, Florian Bruhin wrote:
> * Ron Adam<ron3200 at gmail.com>  [2015-07-19 11:17:10 -0400]:
>> >I had to look at the source to figure out what this thread was really all
>> >about.

And it seems I don't quite get it still, but I am trying.


>> >Basically it looks to me the purpose of adding "assret" is to add an "alias
>> >check" for "unsafe" methods.  It doesn't actually add an "alias".  It allows
>> >a developer to use a valid alias to avoid conflicting with methods starting
>> >with assert that will still work with the mock module.
>> >
>> >The mock module says that when "unsafe" flag is set to True, it will not
>> >raise AssertionError for methods beginning with "assert" and "assret".  It
>> >doesn't specify what "unsafe" means, and why you would want to do that.
>> >
>> >So first do this...
>> >
>> >     * Document "unsafe" in mock module.

I still think documenting the purpose of "unsafe", rather than just the 
effect it has is important.

 From the confusion in this thread, (including my own), it's clear the 
documentation does need some attention.


There are only two places where it mentions "unsafe" in the docs...

The class signature...

"""
class unittest.mock.Mock(spec=None, side_effect=None, return_value=DEFAULT, 
wraps=None, name=None, spec_set=None, unsafe=False, **kwargs)
"""


And in the section below that.

"""
unsafe: By default if any attribute starts with assert or assret will raise 
an AttributeError. Passing unsafe=True will allow access to these attributes.
"""

But that doesn't explain the purpose or why these are unsafe or why it 
should throw an AttributeError.   Or what to do with that AttributeError.


> But if you do a typo, the test silently doesn't fail (because it's returning a
> new mock instead of checking if it has been called):

Do you mean silently passes?


>      >>> m.assert_called()
>      <Mock name='mock.assert_called()' id='140277467876152'>
>
> With the patch, everything starting with "assert" or "assret" (e.g. my
> example above) raises an AttributeError so these kind of issues can't
> happen.

I get that part.  It's checking for a naming convention for some purpose.

What purpose?

    "To raise an AttributeError"  ... but why?


> The thing people are discussing about is whether this should also
> happen if you do "m.assret_called_with()" (i.e. if this is a common
> enough typo to warrant a special exception), or if*only*  things
> starting with assert_* are checked.
>
> The merged patch also treats "assret_*" as a typo, and some people
> think this is a wart/unnecessary/harmful and only "assert_*" should
> raise an AttributionError, i.e. the patch should be amended/reverted.

I think it would be easy enough to revert, It' just a few line in the 
source and docs.  No big deal there.

It's not clear why getting an AttributeError for methods beginning with 
assert is needed, and how that exception is to be used.   (Should it Bubble 
up, or should it be caught and handled?)

Is it just a way to prevent mocks of mocks?  Maybe there is a better way of 
doing that?



>> >If I've still not quite got the gist of this issue, the please correct me.
> This has nothing to do with the assert*keyword*  or optimization -
> only with the behaviour of mock and its "assert_*" methods.
>
> I hope this clears things up!

It clears up that it's not associated with the assert keyword.


Cheers,
    Ron