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

MockContext cannot be reused #441

Closed
SwampFalc opened this issue May 10, 2017 · 4 comments
Closed

MockContext cannot be reused #441

SwampFalc opened this issue May 10, 2017 · 4 comments

Comments

@SwampFalc
Copy link

I was trying out the MockContext in order to test some code I've written.

However, it seems that a MockContext can only be used once, which makes it useless when you have a task that makes more than one Call.

Here's a quick shell session showing the error:

>>> import invoke
>>> mc = invoke.MockContext(run=invoke.Result('success'))
>>> mc.run('one')
<invoke.runners.Result object at 0x7faedea3acd0>
>>> mc.run('two')
Traceback (most recent call last):
  File "<input>", line 1, in <module>
    mc.run('two')
  File "/usr/local/lib/python2.7/dist-packages/invoke/context.py", line 243, in run
    return self._yield_result('_run', command)
  File "/usr/local/lib/python2.7/dist-packages/invoke/context.py", line 236, in _yield_result
    raise_from(NotImplementedError, None)
  File "/usr/local/lib/python2.7/dist-packages/invoke/vendor/six.py", line 718, in raise_from
    raise value
NotImplementedError
@bitprophet
Copy link
Member

I think that's correct, most of its use so far is in one-shot tests in the invocations repo.

I would totally not be averse to it learning how to reset its state after each method call, though - this is just how it was first-drafted and is not an intentional API limitation!

@SwampFalc
Copy link
Author

Okay, so I went looking at the code and later on, I found the actual MockContext documentation.

So with some hindsight, I would say there are a few small issues:

1 - MockContext.run can be a "single Result object, which will be returned once." I believe that the last part violates the principle of least surprise.

From my point of view, providing a single Result should imply that the very same Result keeps getting returned on each call. If you really want the Result to be returned only once, that can be more clearly achieved by MockContext(run=[Result()]). "Explicit is better than implicit."

And I believe this can be achieved simply by deleting line 271 in context.py.

Now, one big caveat: this is from my current point of view where I've just started testing with MockContext. Meaning I don't actually have a serious test written yet. Maybe getting the same Result each time really doesn't make any sense for 'real' tests. So pinch of salt and all that.

2 - The documentation page "Testing Invoke-using codebases" could maybe use one or two additional examples, especially using the different possibilities for the run parameter.

I might have some time over the weekend to write something myself...

@bitprophet
Copy link
Member

Years later (while looking for other MockContext fixes on top one I needed myself) - I'm still surprised that I set up the "returns once only" behavior and even documented it. Most folks will expect Mock-like behavior as you did, and given the problem domain here is "fake remote system state", idempotency is more 'correct' than not.

If I hadn't documented this I'd call it a bug and fix it as-is, but:

  • it's documented
  • as I noted before, I actually doubt too many folks will run into this if they're writing sufficiently narrow tests or otherwise recreating their MockContexts on a per-test basis

So I think the right call is to add an opt-in flag for now and then flip it in the next backwards incompat release. Will try whipping that up.

@bitprophet
Copy link
Member

This will be out in v1.5 as a repeat=True arg to be given to the class constructor 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants