* Ron Adam <ron3...@gmail.com> [2015-07-19 18:06:22 -0400]: > > > On 07/19/2015 02:33 PM, Florian Bruhin wrote: > >* Ron Adam<ron3...@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.
No worries - I'll try to clarify until things are clear :) > >>>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. It's "unsafe" because tests which: 1) are using the assert_* methods of a mock, and 2) where the programmer did a typo (assert_called() instead of assert_called_with() for example) do silently pass. > >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? Yes - it passes without checking the thing the test was intended to check. > > > >>> 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? So you notice you have done a typo and your test will not actually verify what you want it to verify. Compare it with the behavior of a normal object - if you call a method which doesn't exist, it raises AttributeError. This isn't possible with Mock objects, as they are designed to support *any* call, and you can check the calls have happened after the fact. With the patch, if you call any method starting with "assert" which does not exist (assert_caled_with, assert_foobar, assert_called, etc.) this is assumed to be a mistake and you get an AttributeError so you notice there was a mistake. If you pass unsafe=True, you can still call mock.assert_foo() methods as usual and they will return a new Mock, just as calling, say, mock.spam() would. > >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?) Note the discussion *isn't* about the fact that assert-methods should raise AttributeError! The patch also does the same with "assret". At least if I understand things correctly, the discussion is whether *only* "assert*" should be handled as a typo, or "assret*" too. The exception should bubble up, as the whole point of it is to tell you you did a typo and your test is broken. Florian -- http://www.the-compiler.org | m...@the-compiler.org (Mail/XMPP) GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc I love long mails! | http://email.is-not-s.ms/
pgpMO8rT2PffT.pgp
Description: PGP signature
_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
