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

Observing the same mock is not added in the InOrder list

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.

returns the name of the mocked class if specs is set, Dummy Mock otherwise

…ording

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

kaste · 2025-10-05T22:19:40Z

tests/in_order_test.py

+    a.method()
+    b.other_method()
+
+    with pytest.raises(VerificationError) as e:
+        in_order.verify(a).method()
+        in_order.verify(b).other_method()
+    assert str(e.value) == (f"Trying to verify ordered invocation "
+                            f"of {b}, "
+                            f"but no other invocations have been recorded.")


This is likely what to_ignore is for. The error message is wrong because b is not even registered with the InOrder. verifying a is not necessary you just test for unknown/unregistered/unexpected mocks.

tests/in_order_test.py

kaste · 2025-10-05T22:22:54Z

tests/in_order_test.py

+    with pytest.raises(VerificationError) as e:
+        with InOrder([a, b]) as in_order:
+            a.method()
+            b.other_method()
+
+            in_order.verify(a)
+    assert str(e.value) == (f"Some mocks have not been verified: "
+                            f"{in_order.ordered_invocations}")


This shouldn't raise actually. On contraire, we just detach the mocks. Test that verify works after leaving the context manager and (b) that using the mock after detach doesn't get recorded.

kaste

A lot of stuff.

Generally, I want this verify to accept all keyword arguments as the original verify.
I actually had a lot of test cases in my first comment, and I don't see any of them here. Esp. you completely forgot to test different arguments. Like cat.make("meow"), cat.make("rrr") and if they can be verified. Also if verify(cat).say(...) catches all invocations.
The error messages should be more like e.g.

You expected
cat.meow()
but next remembered invocation is
dog.bark()

This is especially hard to do in a helpful way as we don't easily get the names cat and dog. I don't think you should expose the internal Mocks at least as these are also implementation details, in fact proxy objects. The user doesn't see them but only the mocked (wrapped) object.

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.

Already defined in invocation.py

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?

cMancio00 added 3 commits September 23, 2025 23:52

Added Observer abstract class

04b032b

Extended Mock to be observable

e40e66b

Added basic implementation and test case of InOrder

f7e26ea

cMancio00 added 6 commits September 27, 2025 15:22

Removed overrides and fixed typos

0461a32

Fixing flake8

979630b

Fixing flake8

dfdd181

Fixing flake8

38900fd

Fixing flake8

744b463

Mocks can be observed from InOrder

1c4f742

Observing the same mock is not added in the InOrder list

kaste reviewed Sep 27, 2025

View reviewed changes

cMancio00 added 8 commits September 28, 2025 08:14

Adding assertpy as test dependency

f336063

Adding mypy and flake8 to dev-dependencies

1ed8ef8

Raising ValueError when passing duplicated mocks to InOrder

14cb666

Added representation of a mock

3f06876

returns the name of the mocked class if specs is set, Dummy Mock otherwise

InOrder calls are recorded and other mocks calls are ignored from rec…

87c7bac

…ording

Parametrized the Observer to satisfy LSP

1c2795d

Added types stubs for assertpy

9f288b9

If multiple mocks are used, InOrder just remembers the Observed ones

8b4955c

cMancio00 added 2 commits October 5, 2025 08:57

removed assertpy

2f64fd1

Removed mock repr

bb359c6

kaste reviewed Oct 5, 2025

View reviewed changes

tests/in_order_test.py Outdated Show resolved Hide resolved

kaste reviewed Oct 5, 2025

View reviewed changes

cMancio00 added 8 commits October 12, 2025 11:14

removed checks for in_order.mocks as it is an implementation detail

d472006

Better mocks names and removed to_ignore mock as it was confusing

f50ab6f

Better example of what fails in_order verification

f31c3ae

Removed testing unrelated features

21ae2b4

InOrder no longer gets a list of mocks

9c124a7

Improved error message for unexpected mocks and wrong order calls

b9fe384

test that verify can be used after leaving context manager

7bdfe52

Removing definition of RealInvocation.

3769708

Already defined in invocation.py

cMancio00 added 2 commits October 12, 2025 19:09

Fixing flake errors

a431142

Test with multiple arguments

176d13d

cMancio00 requested a review from kaste October 12, 2025 17:19

mocks are detached when leaving context manager

5516646

kaste reviewed Oct 13, 2025

View reviewed changes

mockito/inorder.py

"""

if not (mock in self.mocks):

Copy link

Owner

kaste Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if mock not in self.mocks:

kaste reviewed Oct 13, 2025

View reviewed changes

kaste reviewed Oct 14, 2025

View reviewed changes

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

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

Uh oh!

Inorder implementation #99

Are you sure you want to change the base?

Inorder implementation #99

Uh oh!

Conversation

cMancio00 commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Features

Uh oh!

cMancio00 commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaste commented Sep 24, 2025

Uh oh!

kaste commented Sep 24, 2025

Uh oh!

kaste commented Sep 24, 2025

Uh oh!

cMancio00 commented Sep 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kaste commented Sep 27, 2025

Uh oh!

kaste commented Sep 27, 2025

Uh oh!

cMancio00 commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaste commented Sep 29, 2025

Uh oh!

cMancio00 commented Sep 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaste left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cMancio00 commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cMancio00 commented Oct 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

cMancio00 commented Sep 23, 2025 •

edited

Loading

cMancio00 commented Sep 23, 2025 •

edited

Loading

cMancio00 commented Sep 28, 2025 •

edited

Loading

cMancio00 commented Oct 12, 2025 •

edited

Loading