Writing tests in a program that uses Process.run a lot

Hello everyone,

I’ve been tinkering with a program that uses Process.run, etc in a somewhat crucial capacity. In Ruby, I can use RSpec to expect certain arguments and inject return values in a terse manner, but obviously in Crystal something more appropriate for its static nature is required.

So I decided I’d implement an alternative class called Cmd to be used in my program, mostly so I could safely re-open it in spec_helper:


class Cmd < Process
  @@ran = [] of Tuple(String, Array(String))
  @@yield_process : Nil | SpecProcess

  def self.ran
    @@ran
  end

  def self.run(**args : Process.run)
    @@ran << args
  end

  def self.spec_yield_process(value)
    @@yield_process = value
    yield
    @@yield_process = nil
  end

  def self.run(*args)
    @@ran << args
    yield @yield_process
  end

  def self.spec_reset
    @@ran.clear
  end
end

It works okay:

describe Grub, "#run_mkconfig" do
  before_each do
    Cmd.spec_reset
  end

  it "runs it on a path" do
    Grub.new.run_mkconfig("/test/path")
    Cmd.ran.should eq([{"/usr/sbin/grub2-mkconfig", ["-o", "/test/path"]}])
  end
end

Now I want to write something to allow me to assert something about the args that go into Cmd and inject its stderr. Something like this:

      Cmd.run("ls", ["/doesnotexist"], error = Process::Redirect::Pipe) do |p|
        p.error.gets_to_end
      end

But trying to compile specs against this yields something like this:

In spec/spec_helper.cr:29:11

 29 | @@ran << args
            ^-
Error: no overload matches 'Array(Tuple(String, Array(String)))#<<' with type Tuple(String, Array(String), Process::Redirect)

OK, sensible enough. I suppose I could copy in Process.run's arguments into a NamedTuple and that’d be fine, but I thought to pause here and ask:

  1. Is there a better way to refer to the tuple/named tuple of a method’s parameters than copying it?
  2. Do you have any ideas for performing this kind of injection & assertion in general?
  3. If I copy the entire Process.run parameter-struct, is it going to make my assertions unwieldy?

Thank you for your considerations.

The Cmd class with class vars is not a good idea for testing, better using instance variables.
This way, you’ll have individual tests with no possible side-effects.

Also, the unit tests should test methods, and their results. The fact that processes are spawned is an implementation detail and should not matter.

What would you like to test in reality with the "runs it on a path" test? Maybe the real check is verifying the value of the file at /test/path?

@fdr I agree with @j8r that using class variables may be limiting you here. Would something like this work?

class Cmd(Args, KWArgs)
  def self.run(*args, **kwargs)
    new(args, kwargs)
  end

  def initialize(@args : Args, @kwargs : KWArgs)
  end
end

pp Cmd.run("ls", %w[*.cr])
# #<Cmd(Tuple(String, Array(String)), NamedTuple()):0x10f4cae60
#  @args={"ls", ["*.cr"]},
#  @kwargs={}>

pp Cmd.run("ls", %w[*.cr], error: Process::Redirect::Pipe)
# #<Cmd(Tuple(String, Array(String)), NamedTuple(error: Process::Redirect)):0x10f4d2d80
#  @args={"ls", ["*.cr"]},
#  @kwargs={error: Pipe}>

The idea is that the types of the args and keyword args are inferred so the tuple lengths and types can be more flexible. I imagine that a little more work would need to be done to make Cmd a drop-in mock for Process, but using instances rather than class-level state can make your code a bit more flexible.

I use this style in my code a bit and, depending on how flexible you make your test class, it is possible to get a green test run even when the class used in production won’t conform to that particular API. However if that’s true the code won’t compile, so the probability of deploying busted code like that is quite low.

Why? Are the tests run in parallel? I bit the bullet and just copied out the signature from Process.cr, as I notice it spells it out a few times, presumably there’s not an easier, better way to write that. It’s okay. I changed things so that one is obliged to invoke a block to use the test variant of Cmd in this way.

I could go to using to instance variables, but it doesn’t seem to really offer anything but more notation.

That’s an interesting approach. I’ll think about it. So the problem is rather more extensive than this, because I’d also like to inject stderr, stdout, and return codes, and handle the block format of run. I could convert my stuff to use instance variables in this style and be spared spec_reset.

Given Process.cr repeats itself a few times as far as the parameters go, I think it’s okay to do so a few more times for a test.

class Cmd
  alias ProcessNamedTuple = {command: String, args: Array(String), env: Process::Env, clear_env: Bool, shell: Bool, input: Process::Stdio, output: Process::Stdio, error: Process::Stdio, chdir: String?}
  @@ran = [] of ProcessNamedTuple

  @@yield_process : Nil | SpecProcess

  def self.ran
    @@ran
  end

  def self.spec(status : Process::Status)
    @@return_status = status
    yield
    @@return_status = nil
  end

  def self.spec(process : SpecProcess)
    @@yield_process = process
    yield @@yield_process
    @@yield_process = nil
  end

  def self.run(command : String, args = nil, env : Process::Env = nil, clear_env : Bool = false,
               shell : Bool = false, input : Process::Stdio = Process::Redirect::Close,
               output : Process::Stdio = Process::Redirect::Close,
               error : Process::Stdio = Process::Redirect::Close,
               chdir : String? = nil) : Process::Status
    @@ran << {command: command, args: args, env: env, clear_env: clear_env, shell: shell, input: input, output: output, error: error, chdir: chdir}
    fail "BUG" unless status = @@return_status
    status
  end

  def self.run(command : String, args = nil, env : Process::Env = nil, clear_env : Bool = false,
               shell : Bool = false, input : Process::Stdio = Process::Redirect::Close,
               output : Process::Stdio = Process::Redirect::Close,
               error : Process::Stdio = Process::Redirect::Close,
               chdir : String? = nil)
    @@ran << {command: command, args: args, env: env, clear_env: clear_env, shell: shell, input: input, output: output, error: error, chdir: chdir}
    yp = @@yield_process
    yield yp if yp
  end

  def self.spec_reset
    @@ran.clear
  end
end

Still rough but it seems to have all the power and structure necessary.