Skip to content

Conversation

@SwampFalc
Copy link

A small pull request to make MockContext's .run() and .sudo() return Results with the command attribute set.

From my own use cases, the command attribute is the only one I ever seem to need. I frequently create a layer of simple tasks, upon which I create more complex tasks that will call the others. Proper unit testing requires me to check that all parameters are properly passed to the lower tasks, ie. that they contruct the right command for the ctx.run().

Therefore, in this use case, I cannot provide command to the Result() constructor and I need some way to get at the actual command so I can test it.

So I wrote these small modifications to the MockContext.run() and MockContext.sudo() methods. If a command is given to the Result() constructor, it is of course retained, otherwise the actual command is injected.

While I believe that this is the most likely use case for such an injection, I would keep the TODO remarks about maybe one day injecting other args/kwargs as well, as I don't believe we can be 100% certain those will never be useful...

@bitprophet
Copy link
Member

Thanks for reporting this, it's a weird oversight! I found a cleaner way to fix it (and wrote tests) but am crediting you in the changelog despite not merging this directly.

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 this pull request may close these issues.

3 participants