Mock ShellScript within tests
jefpij opened this issue · 8 comments
Describe the problem
I have an application with multiple layers (ui, interactor, repository, service). The ui layer; although cmdline, is the layer where I use ShellScript. I want each layer to be testable independently which means layers or their constructor arguments will need to be mocked in order to keep it as isolated as possible. Shellscript, being an final class with internal constructor, cannot be given passed as a constructor argument. This makes writing isolated unit tests impossible in my setup.
Describe your solution
Appending the keyword 'open' will solve this.
Checklist
- [ x ] I've read the guide for contributing.
- [ x ] I've checked there are no other open pull requests for the feature.
- [ x ] I've checked there are no other open issues for the same feature.
Additional context
None
Pull request can be found: https://github.com/lordcodes/turtle/pull/98/commits
In this case, you can create a wrapper class within your project around ShellScript and can then mock that instead of ShellScript directly, rather than changing the library code to make something open unnecessarily.
Its also worth saying that I use Mockk in my Kotlin projects where I mock non-open classes. If it is necessary there is also the All Open Kotlin compiler plugin.
I will close this for now, as for this use-case no change is needed to the library.
Thank you for reaching out!
I've attempted to create a wrapper around it but am not succeeding. Would you be willing to give a pointer in the right direction perhaps?
I've created a small snippet to demonstrate where I am stuck and concluded that the "open" keyword was required.
import com.lordcodes.turtle.shellRun
import java.io.File
class ShellScriptWrapper(val workingDirectory: File? = null, script: ShellScriptWrapper.() -> String) {
fun command(command: String, arguments: List<String> = listOf()) {
shellRun(workingDirectory) {
command(command = command, arguments = arguments)
}
}
}
So the general plan is to inject this ShellScriptWrapper as a constructor parameter in my layers to make it mockable. The problem I am facing is that by using this approach I can only use ShellScript through the shellRun method. ShellScript has an internal constructor so I cannot create an instance of it within my wrapper.
Being forced to use shellRun, a new instance of ShellScript will be made for each call to command. Needless to say this isn't ideal when wanting to execute multiple commands.
Ideally I would want to create an internal ShellScript instance within my wrapper class and use it with the command-method for instance.
In addition when for instance wrapping ShellScriptWrapper.command() around ShellScript.command(), there is also use of specific callbacks (ProcessCallbacks). Would you recommend wrapping an interface around that interface as well as to uphold separation?
Thank you.
That's a very good point, that the ShellScript constructor is internal. I think the main issue here is that the library was only really designed to be used to quickly and easily run commands, i.e. via 'shellRun'. I hadn't forseen that people would want to use the ShellScript instance directly as you are.
I don't see any problem with making the ShellScript constructor publicly accessible, so that consumers of the library can use shellRun or ShellScript. However, it is worth saying the ShellScript object itself is very lightweight, it doesn't store any state as such and doesn't involve anything expensive to create.
With regards to the wrapper I would just give your wrapper functions that call through to ShellScript (or shellRun for now until constructor is accessible). With regards to how you handle callbacks that's completely up to you.
I like the convenient shellRun functionality a lot. Having to create a wrapper around the shellRun to actually wrap ShellScript doesn't produce the best readable setup, to be honest. It also takes away the ease of use as demonstrated with this library.
Furthermore I agree with the rest.
I noticed you've already updated the code base.
Thank you and great job on the library!
Yes I agree. 'shellRun' is designed for use directly really, to run commands. For example, I use it within custom Gradle tasks.
However, as you said when use within a codebase such as you are describing, it is better to use ShellScript so you have a little more control and in this case the constructor really needed to be public, which has been merged.
I will aim to get a release out as soon as I can.
Thank you, I am very glad to hear it is useful for someone 😄
Thank you!