Inorder implementation #99

cMancio00 · 2025-09-23T22:03:32Z

This is the PR for #98.

The idea is to make Mock observable so we can register ordered invocations from different mocks.
The mocking_registry helps to get the mocks for comparing.

Features

Mocks should be observable from an InOrder object to register the order of invocations
Trying to observe the same Mock more than once should be fine
Should work with specs and no-specs Mocks
~~Should work with spy objects (Maybe in an other PR)~~
Mocks should be detached automatically with a context manager and also when the Queue call is empty
Verification errors should be meaningful

@kaste Feel free to add elements to this list

cMancio00 · 2025-09-23T22:06:15Z

I should not use @override annotation since is not supported before Python 3.12 and also use mypy locally first.

kaste · 2025-09-24T09:15:22Z

Very promising.

For the tests:

Don't do the friends things if it only hides the actual usage of the mocks in the tests.
Maybe even the setup is limiting here. Sometimes we test a specced mock, sometimes
not.

cat = mock()
in_order = InOrder([cat])
cat.meow("HI")
cat.meow("HO")

a)
in_order.verify(cat).meow("HO")  # raises

b) 
in_order.verify(cat).meow("HI")  # passes
in_order.verify(cat).meow("HO")  # passes
in_order.verify(cat).meow("HO")  # raises, no?

b1) 
in_order.verify(cat).meow("HI")  # passes
in_order.verify(cat).meow("HO")  # passes
in_order.verify(cat).meow("HI")  # raises

c)
in_order.verify(cat).meow(...)  # passes
in_order.verify(cat).meow(...)  # raises

d)
in_order.verify(cat).bark(...)  # raises

implement context manager to "detach" automatically.

cat = mock()
with InOrder([cat]) as in_order:
    cat.meow("HI")
cat.meow("HO")

...

Check the actual error messages in the tests. The most important part is to
have good error messages. E.g. user expected bark(...) but next invocation was
actually meow("HI").

kaste · 2025-09-24T09:16:37Z

(I usually do the tests first. E.g what behavior do I even expect for all the cases. Do they all make sense in concerto.)

kaste · 2025-09-24T09:20:40Z

a)
probably valid and we have two separate "lines" of recorded executions
with InOrder(cat) as a, InOrder(dog) as b:

b)
a = InOrder(cat)
b = InOrder(cat)   # raises? already registered

cMancio00 · 2025-09-27T13:16:39Z

Don't do the friends things if it only hides the actual usage of the mocks in the tests.

Okay, I'edit the set up to use a function that calls both the interfaces

Maybe even the setup is limiting here. Sometimes we test a specced mock, sometimes
not.

I'll also test non specs mocks

(I usually do the tests first. E.g what behavior do I even expect for all the cases. Do they all make sense in concerto.)

I'll try to implement it in TDD

a = InOrder(cat)
b = InOrder(cat) # raises? already registered

I thought that if we use a set as a container for observed subjects we can not have problems with already registered objects and the scope of an InOrder object is just in a single test case

tests/in_order_test.py

kaste · 2025-09-27T17:13:49Z

mockito/mocking.py

+
    def remember(self, invocation):
        self.invocations.append(invocation)
+        self.notify()


it reads much better when we could do, for each observer. oberserver.notify(invocation). This is semantically equivalent to: for each observer. observer.remember(invocation). As if the invocations here are a specialized variant of it.

I'll look in to it.

Still: for each observer. observer.remember(invocation)

tests/in_order_test.py

mockito/inorder.py

kaste · 2025-09-27T17:23:14Z

cat = mock()
dog = mock()

with InOrder(cat) as in_order:
    dog.bark()
    cat.meow()

in_order.verify(dog).bar()  #  raises

kaste · 2025-09-27T17:38:12Z

a = InOrder(cat)
b = InOrder(cat) # raises? already registered

Yeah, that should probably not raise. Nesting should ideally work, we need to ensure instead that nesting works.

cMancio00 · 2025-09-28T14:14:28Z

Added a testing library to express better assertions with containers...I'm working in the various steps
@kaste Isn't sufficient to use

rye add --dev assertpy

kaste · 2025-09-29T10:08:34Z

Generally, this project predates any of the modern tools, like rye, uv etc.

Don't do these distracting things here. They don't help you implement the actual thing.

For this new feature, just use pytest and plain asserts. You don't even need to subclass unittest.TestCase. Look at numpy_test.py as an example. Do not write test around in_order.mocks and in_order.ordered_invocations (=remembered_invocations). These are likely implementation details. Users shouldn't need those. Again: Try to use dumb mocks. Inline everything, don't be DRY here.

Already think about using InOrder(*mocks) to support the shorter and easier to write InOrder(mock()).

cMancio00 · 2025-09-29T11:20:28Z

Generally, this project predates any of the modern tools, like rye, uv etc.

Don't do these distracting things here. They don't help you implement the actual thing.

For this new feature, just use pytest and plain asserts. You don't even need to subclass unittest.TestCase. Look at numpy_test.py as an example. Do not write test around in_order.mocks and in_order.ordered_invocations (=remembered_invocations). These are likely implementation details. Users shouldn't need those. Again: Try to use dumb mocks. Inline everything, don't be DRY here.

Already think about using InOrder(*mocks) to support the shorter and easier to write InOrder(mock()).

I don't get it, Unit Tests should help me implementing the feature and serves other developers as a sort of documentation. Users should not reads our tests, but the documentation and the example in it (like doc tests). (In an extremis we can provide E2E tests with behave to get the high level functionality of a feature.
The assertion library that I wanted was to have better readability in the test cases and to have functions to have better tests while working with containers.

Anyway the repo is your so I'll do what you want. I'll remove the dependency, use dumb mock, inline everything (no setup like xUnit) and write the tests just with documentation purpuse

cMancio00 · 2025-10-05T08:39:03Z

@kaste Hello, what do you think?
I still need to implement the context manager and the unstub.

Do you think that we should support a workflow like this?:

  a = mock()
  b = mock()

  when(a).method().thenReturn("Calling a")
  when(b).other_method().thenReturn("Calling b")

  in_order: InOrder = InOrder([a,b])
  a.method()
  b.other_method()
  a.method()

  in_order.verify(a).method()
  in_order.verify(b).other_method()
  in_order.verify(a).method()

For now it is not supported.

Best regards

kaste · 2025-10-05T09:53:35Z

Isn't that the meat of the re-implementation of InOrder?

cat = mock()
dog = mock()

when InOrder(cat, dog) as in_order:
    cat.meow()
    dog.bark()
    cat.meow()

in_order.verify(cat).meow()
in_order.verify(dof).bark()
in_order.verify(cat).meow()

The old InOrder supported only one mock, and your implementation supports proper cross-mock interactions?

kaste · 2025-10-05T11:31:53Z

I pushed master to update to modern tooling (uv) and generally towards v2. Please try a rebase. E.g. RememberedInvocation | RememberedProxyInvocation is now invocation.RealInvocation.

cMancio00 · 2025-10-05T14:15:06Z

The old InOrder supported only one mock, and your implementation supports proper cross-mock interactions?

My implementation supports cross-mock interaction like

in_order.verify(cat).meow()
in_order.verify(dof).bark()

but doesn't yet supports an interaction of this type:

in_order.verify(cat).meow()
in_order.verify(dof).bark()
in_order.verify(cat).meow()

Where a mock method is called twice (or more) in different orders.
But this is easy to fix (I think). After checking that the specific instance of a mock is called I delegate to the original implementation of verify with times=1. I think changing it to atleast=1 solves the issue.

I pushed master to update to modern tooling (uv) and generally towards v2. Please try a rebase. E.g. RememberedInvocation | RememberedProxyInvocation is now invocation.RealInvocation.

I'll looking into that

cMancio00 · 2025-10-05T16:05:46Z

@kaste I' happy with the implementation and tests. Let me know if you need any changes or some documentation to put in the docs with examples (and how you wanted).
Best regards

P.S.
I would like to help in an other PR with docs (even for all the project) and "fix" spy objects. If you also need help with the CI let me know.

kaste · 2025-10-05T22:03:26Z

mockito/inorder.py

+    def mocks(self):
+        return self._mocks
+
+    def update(self, subject: Mock) -> None:


-> remember(self, invocation: RealInvocation) -> None

Why you haven't done this?

kaste · 2025-10-05T22:04:42Z

mockito/inorder.py

+        :param subject: subject to be added to the list of ordered invocation
+        """
+        self.ordered_invocations.append(
+            (subject, subject.invocations[-1])


subject would have been too generic anyway, but subject.invocations[-1] is an implementation detail. Just pass the invocation in. self.invocations: deque[RealInvocations]

kaste · 2025-10-05T22:05:55Z

mockito/mocking.py

+RealInvocation = Union[
+    invocation.RememberedInvocation,
+    invocation.RememberedProxyInvocation
+]


RealInvocation is actually defined in invocation.py

tests/in_order_test.py

kaste · 2025-10-05T22:13:04Z

tests/in_order_test.py

+    a = mock()
+    b = mock()


Don't use a and b as names. Use cat and dog or some other names.

tests/in_order_test.py

kaste · 2025-10-05T23:01:49Z

tests/in_order_test.py

+                            f"Called {mock_registry.mock_for(a)}, "
+                            f"but expected {mock_registry.mock_for(b)}!")


To get a feeling for the error messages, these should be typed out. mock_for returns an internal helper anyway. So that must rather be You expected an invocation on b but the next invocation comes from a.

cMancio00 · 2025-10-12T17:09:08Z

Also if verify(cat).say(...) catches all invocations.

What is the purpose of operator ...?

cMancio00 · 2025-10-12T17:19:07Z

Things I can't do

Make the sign of inorder verify the same as verify because i delegate to the original with fixed arguments so changing arguments will lead to unwanted behavior
Things I don't understand from the past review

kaste · 2025-10-13T23:43:36Z

mockito/inorder.py

+
+        """
+
+        if not (mock in self.mocks):


if mock not in self.mocks:

kaste · 2025-10-13T23:48:29Z

mockito/inorder.py

+            raise VerificationError(
+                f"InOrder verification error! "
+                f"Wanted a call from {str(expected_mock)}, but "
+                f"got {invocation} from {str(called_mock)} instead!"


I asked you to not expose our internal Mock class. Users only care about their mocks and actually their real classes. (Actually most of the time, user just patch their real classes and don't use "mocks" at all.) If you have a Mock, most likely the user is interested in its mocked_obj and/or its spec. Our "Mock" has a proxy relation to the mocked object.

kaste · 2025-10-13T23:50:34Z

mockito/mocking.py

+    def __str__(self):
+        name: str = 'Dummy'
+        if self.spec:
+            name = self.spec.__name__
+        return f"Mock<{name}>"


Likely not needed as Mock is an internal class, the standard repr implementation Python provides for free was always enough.

kaste · 2025-10-13T23:51:35Z

mockito/mocking.py

+        def __str__(self):
+            name = 'Dummy'
+            if spec:
+                name = spec.__name__
+            return f"Mock<{name}>"
+


Don't override the __repr__ just above that. Why is that useful?

kaste · 2025-10-13T23:52:30Z

tests/in_order_test.py

+    with pytest.raises(ValueError) as e:
+        InOrder(a, a)
+    assert str(e.value) == ("The following Mocks are duplicated: "
+                            "['Mock<Dummy>']")


Here you see that your new str implementation for mock isn't helpful as it is basically a static string for all dumb mocks.

kaste · 2025-10-14T00:01:05Z

tests/in_order_test.py

+        "Wanted a call from Mock<Dummy>, but got "
+        "bark() from Mock<Dummy> instead!"


These come from the Mock.__str__ implementation. Please note that this error message is not helpful for the end-user at all.

Ideally maybe

Wanted but not invoked cat.meow() Next recorded invocation is: dog.bark()

Is that too tricky?

Then maybe

Wanted a call from ... Next recorded invocation is ...bark()

That shouldn't be that hard.

kaste · 2025-10-14T00:03:27Z

tests/in_order_test.py

+def test_exiting_context_manager_should_detatch_mocks():
+    cat = mock()
+    dog = mock()
+
+    with InOrder(cat, dog) as in_order:
+        cat.meow()
+        dog.bark()
+
+        in_order.verify(cat).meow()
+        in_order.verify(dog).bark()
+


The important thing is that invocation on cat do not get recorded after detach.

kaste · 2025-10-14T00:09:56Z

mockito/inorder.py

+        if not self.ordered_invocations:
+            raise VerificationError(
+                f"Trying to verify ordered invocation of {mock}, "
+                f"but no other invocations have been recorded."


Why "no other invocations".

Maybe

Wanted a call on ... But nothing has been recorded yet.

kaste · 2025-10-14T00:28:24Z

mockito/inorder.py

+        expected_mock = mock_registry.mock_for(mock)
+        if called_mock != expected_mock:
+            raise VerificationError(
+                f"InOrder verification error! "


Generally, do not start with "InOrder verification error!" but just a newline "\n".

kaste · 2025-10-14T00:31:21Z

mockito/inorder.py

+                f"Trying to verify ordered invocation of {mock}, "
+                f"but no other invocations have been recorded."
+            )
+        ordered_invocation = self.ordered_invocations.popleft()


Instead of popping them you can maybe take the next invocation that is still not .verified. IIRC verify(_main) sets that.

kaste · 2025-10-14T00:36:47Z

What does it take to support the full signature of the original verify function?
... means: any arguments
What "things" did you not understand?

kaste · 2025-11-18T12:08:50Z

Difficult to think this through... The standard verify should be supported at least.

Authored-by: Christian Mancini <[email protected]> Co-authored-by: herr kaste <[email protected]>

... still doesn't make a good error message

kaste reviewed Sep 27, 2025

View reviewed changes

kaste force-pushed the master branch 5 times, most recently from 26ca517 to 70a7c8a Compare October 5, 2025 11:26

cMancio00 marked this pull request as ready for review October 5, 2025 16:00