Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Write a recommendation for asserting multiple things on a signal #325

Closed
DominicOram opened this issue May 22, 2024 · 4 comments · Fixed by #335
Closed

Write a recommendation for asserting multiple things on a signal #325

DominicOram opened this issue May 22, 2024 · 4 comments · Fixed by #335
Assignees

Comments

@DominicOram
Copy link
Contributor

As a developer I may want to check multiple things on a mocked signal. Currently we have assert_mock_put_called_with but what if I want to assert many calls on the mock? We can currently do this with something like:

def call_another_mock(call_log: MagicMock):
    def _call_another_mock(value, **kwargs):
        call_log(value, **kwargs)

    return _call_another_mock

def test():
    call_log = MagicMock()
    callback_on_mock_put(my_device, call_another_mock(call_log))
    call_log.assert_has_calls(_call_list((5.3375, -3.55, 2.43, 48.974, 15.8)))

This is fine except:

  • There is already a mock internally that we could use
  • Adds downstream boilerplate

Some other ways we could do it are:

  • Allow access to the mock:
def get_mock(signal: Signal) -> Mock:
    backend = _get_mock_signal_backend(signal)
    return backend.put_mock
  • Add a def assert_mock_has_calls(signal: Signal, calls) - I'm not a big fan of this as it seems a slippery slope to re-implementing mock

Acceptance Criteria

@callumforrester
Copy link
Contributor

Allowing access to the mock seems like a good idea to me.

@coretl
Copy link
Collaborator

coretl commented May 22, 2024

Two opposing suggestions:

  1. Remove assert_mock_put_called_with, and replace with get_mock_put(signal). The only downside is the usual problem with mock, if I type mock.assret_has_calls(calls) (as I have sometimes done) then the test passes as the mock just gives me back another mock and calls it with calls. I don't know if there is a way to get it not to do this?
  2. Add assert_mock_put_has_calls and assert_mock_put_not_called and all the other ones

I think I prefer 1., but I would love it if there was a way to catch the typo issue...

@DominicOram
Copy link
Contributor Author

DominicOram commented May 22, 2024

Yh, 2 is a slippery slope. I'm happy to go 1. I think you can avoid the typo problem by using spec, e.g.

class MyClass():
    def blah(self):
         ...

Mock(spec=MyClass) # This can only be called with blah() or the methods inherent to Mock

I'm happy to look at this as part of this issue?

@coretl
Copy link
Collaborator

coretl commented May 23, 2024

I'm happy to look at this as part of this issue?

Yes please!

@DominicOram DominicOram self-assigned this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants