We had a discussion on the list sparked by the assret checking, and in it I proposed that the API would be cleaner if the asserts were module functions.

e.g. rather than::

 a_mock.assert_called_with(Foo)


 assert_called_with(a_mock, Foo)

Michael has objected to this saying that the current structure is part of mock's success - but I'm filing this since a number of other core devs seemed to really like the idea. We can discuss here and if the consensus is that it wouldn't be an improvement - thats fine. OTOH if the consensus is that it is an improvement, this can serve as a memo to someone to implement the new API.

As a frequent and long-time user of mock, the `assert_*` methods being on the mock object itself has always struck me as an unfortunate wart on an otherwise great library. The change to raise `AssertionError` on `assert_*` and `assret_*` feels like piling an ugly band-aid on top of the wart. I'd love to see Robert's suggested solution of moving the assertion helpers to standalone functions.

Er, I meant `AttributeError`, of course...

What about other methods/properties like called, call_count, and reset_mock? It seems that they should be removed as well to be consistent with the reason for this change.

Not sure it's my place to comment here, but here are my 2 cents: I think Robert's proposal to have module functions is the only way to have a user-friendly and robust API, and it solves more than just the assert typo problem. (And yes, it would require moving the mock public API entirely to functions/classmethods).

To me, there's an underlying fragility in mock since the current API for `Mock` is not cleanly separated from the mocked API. This issue creates the problem of the  assert typos, and also creates problems with name conflicts (I've always thought the `.call_count` attribute was particularly likely to be clobbered).

The only bullet-proof way I can think of to ensure such a conflict does not take place is to separate the namespaces altogether, by moving the data out of the Mock object and into a global structure. E.g., `mock.Mock` could have a class attribute (say `mock.Mock.call_log`) tracking all of the calls to all mocks, and there could be a series of classmethods to query this store. Unfortunately, this design goes seriously against the grain of OOP, but we're essentially back to Robert's proposal.

A more OOP-friendly approach sacrifices the '100% clash-proof guarantee' and only provides a 'highly unlikely to clash' guarantee instead: Mangle the mock API namespace. Mock currently does this for its internal attributes (e.g., `._mock_parent`) but not for its public API (e.g., `.assert_called_once_with`). To remain user-friendly, of course we wouldn't require users to mangle names by hand, but would provide convenience functions to access these mangled attributes/methods, which puts us right back at Robert's proposal.

I'm not wild about this idea. The problem with the assert methods has *essentially* been solved now, so I'm not convinced of the need for this change (unless users really *need* to have their own mocked attributes like "assert_called_with" which I think is highly unlikely).

Part of the genius of mock was providing a flexible mock object that also encapsulated simple methods for introspecting/asserting how it has been used. Changing to require users to import/know about a whole host of separate functions doesn't feel like an improvement to me. That's aside from the whole "breaking people's code for no tangible benefit" issue.

I acknowledge that other people, Carl for example, have different opinions - but from talking to many, many mock users over the years I think that those with the philosophically purist approach are in a minority to those who appreciate the more practical approach that mock takes.

FWIW, my assumption was that the typical usage pattern would be `import mock` rather than separate imports of all the assertions, so I don't think there'd really be an increase in what users need to know about or import. (They already need to know about the methods on the mock object, knowing about functions in a module isn't practically any different.)

But the "breaking working code for insufficient benefit" argument is strong indeed, and personally I'm happy to defer to the module author on this one.

Thanks for writing and maintaining mock!

Some great points raised here.

Yes, we'd need .calls and .returnvalue and so on all made accessible via another route, and that would be a less direct spelling.

e.g. a_mock.returnvalue = 3
-> 
# function
set_returnvalue(a_mock, 3)
or
# separate object to get at the mock housekeeping vs the mocked interface
mocki(a_mock).returnvalue = 3

etc.

None of which make me want to say 'wow thats a great api'.

I wouldn't want to advocate for a patch for this unless:
 - it would feel as convenient to use as the current API does
 - it would be possible to be both forward and backwards compatible with the existing API indefinitely: There are lots of users of mock, and offering a [presumably] improved API is not worth turning all that code into an instant sea-anchor: if we're going to change this, we need to do so in a -very- graceful fashion, because unlike e.g. the mock_open things that only affect some users of mock, this would (if done poorly) break every user ever.

For clarity, I have no plans to write such a patch at this time, though if someone did come up with a proof of concept I'd happily review it, and dig up Michael to get his thoughts *if and only if* it met the two constraints above.

I'm not sure of the right bug tracker state to say 'this is a maybe, maintainer isn't enthused about it'.

As I wrote (late) in the thread that shall not be named, I don't think it makes any sense to have magic assert_ methods on the Mock object.  Not only does the "magic" clearly lead to too many ambiguities and other problems--I think they make less sense from an API standpoint in the first place.  Typically asserting something in a test is not something an object *does*--a method.  More often we as a test writers assert something *about* an object.  The assertion is an external tool meant to measure and introspect things about the system under observation.  In this case, although Mock is itself a testing tool, we're introspecting something about the Mock object as external observers.

Regarding the points raised about accessing other attributes like .calls and .returnvalue, and Robert's idea about set_returnvalue make me think that this could be softened somewhat.  Michael is right that the assert_ problem is mostly solved now.  Though I still think special casing one way of mistyping assert is the wrong way to go, and I feel like the "unsafe" option is more of kludge than anything, though it mostly gets the job done.

So rather than go whole hog and say that all introspection on and interaction with the Mock object should be via functions (which I agree would weaken much of its convenience), only assertions should be via functions.

As a recent user of the mock package I've found it strange, even before this controversy, that I called assert_<foo> as *methods* on the mock object itself.  In all past cases when I've wanted to write special assertions for my classes I've written those as functions. In the case of Mock, using assert functions would have none of the spelling issues, and would be quite easy to drop into any test.  It might also open up possibilities with duck-typing, though I'm not sure to what end yet...

TL;DR, I think keep the existing assert_ methods for backward compat, including the unsafe flag and assret check, but add assert_ functions for those who would prefer such an API, and promote them in the docs as safer w.r.t. misspellings, and ensuring that your assertions are called.
If it would help sweeten the deal, the assert_ functions could even come as staticmethods on the Mock class, so as to reduce the effort in writing import statements.

I had a chance to talk to Guido about this at EuroPython. His feeling was that a separate set of functions *is* a better API - or at least would have been if mock had been designed this way from the start. 

However, the most serious problem with the current API is solved (as assert* attributes that don't exist now raise an AttributeError) and it isn't worth breaking people's code by deprecating then removing the assert methods just for a slightly cleaner API.

As I agree about breaking code I'm inclined to close this with WontFix unless someone has strong feelings that it should be left open.

(Thanks Robert for the proposal - it's hard for me not to feel emotionally attached to the current API, which makes it correspondingly hard to be objective about it, but I appreciate people's desires to improve mock.)

Would it be possible to add the functions and *not* deprecate assert_ methods?

The existing solution is still acknowledged to be an incomplete solution.  It is still possible to make other typos that result in unintentional non-assertions (it's also not entirely clear from the docs when one would want to set unsafe=True, though at least the default unsafe=False is better).  I'm not against the existing solution--I think it's reasonable and that calls of "special cases yadda yadda" are unfair. Sometimes practicality beats purity.

So, although adding assert_ functions would be a bit redundant, it's also guaranteed safer, and so probably worth making an exception for.  

There's a tradeoff here between convenience and robustness and I think we can have it both ways.  If I were to write new tests using Mock I would probably define such functions myself, but that would get repetitive quickly.

I like the idea of having the new module functions without deprecating the old ones. Then projects can encourage use of the API on their own terms. Maybe there could even be a global that projects could set that would disable the object methods...

I don't mind if this is open or closed: we've now got a clear set of requirements *should* someone want to work on it::

 - it would feel as convenient to use as the current API does

 - it would be possible to be both forward and backwards compatible with the existing API indefinitely: There are lots of users of mock, and offering a [presumably] improved API is not worth turning all that code into an instant sea-anchor: if we're going to change this, we need to do so in a -very- graceful fashion, because unlike e.g. the mock_open things that only affect some users of mock, this would (if done poorly) break every user ever.


I think its possible to deliver that systematically across the API (not just the asserts - the whole thing) - but its nontrivial.

As for the assert misspellings raising AttributeError -- that's not true of all of them, only those starting with "assret" or "assert" [1].

In my company, we recently cleaned up a bunch of other assert-misspellings, including cases where a real bug was masked in the code by that. That's why I filed https://bugs.python.org/issue41877.

IMO, the fact that the assert-misspellings issue is not completely fixed seems to increase the motivation to add a module-level assert.

While it's easy to add further misspellings to the check in [1], the general solution proposed in this ticket avoids a length whack-a-mole for code owners who would be able / willing to switch to a module-level assert.

[1] https://github.com/python/cpython/blob/master/Lib/unittest/mock.py#L634