Unit Test of method calling system() - how?

How can I unit test the two methods:

print_usage_full_and_exit

and

print_usage_short_and_exit

http://pastie.org/953804

Possibly, I need to change the code into something more testable!

Ideas most welcome!

Martin

On 5/10/10, Martin H. [email protected] wrote:

Possibly, I need to change the code into something more testable!

Ideas most welcome!

I would extract a helper from both methods which does the bulk of the
work (the first 2 lines) but does not exit. Then test the helper and
don’t bother testing the methods that call it.

You then have the problem that this print_wiki command is printing
something to stdout, which you want to test. Either capture stdout in
the test and verify it contains what you want, or use backticks
(...) instead of system in your helper method.

Since print_wiki might change its behavior, you might think about
making the assertion that tests print_wiki’s output string somewhat
loose… just verify that some expected substring occurs in there
somewhere, rather than verifying that it is exactly equal to some
expected string.

$0 is also going to present you some problems in a test… it will not
be the same value in the test as it is when run from your main
program. Not sure what to suggest about that without more context.

3 other points:

looks like both methods have an unintended extra trailing + on their
first line

standard style is to use 2 spaces for indentation; you appear to be
using 8

since the 2 methods are nearly identical, you should pull out the
common code.

On Mon, May 10, 2010 at 5:05 PM, Martin H. [email protected] wrote:
The basic idea is to intercept system, a very crude first approach is

def reset
$mysys = []
end
def system *args
$mysys << %x{#{args.join( " " )}}
end

then you can capture system calls outputs, you might want to add some
code to capture exit values from system calls. I do not know how to do
both out of my head, probably using a rescue cause.

HTH
R.

@Caleb

I would extract a helper from both methods which does the bulk of the
work (the first 2 lines) but does not exit. Then test the helper and
don’t bother testing the methods that call it.

Hm, I thought all this unit testing was about testing the interface -
these are private methods. But perhaps this is one of those rare cases
where you have to do all sorts of tricks? I was wondering if a special
class for printing usage would be in order?

You then have the problem that this print_wiki command is printing
something to stdout, which you want to test. Either capture stdout in
the test and verify it contains what you want, or use backticks
(...) instead of system in your helper method.

Actually, ‘print_wiki’ outputs to stderr - which makes sense if you are
executing a command and piping the output to a file. You want to see the
usage info as an indicator that your did something wrong (if you did
mess up), and not have the usage information in the output file!

Since print_wiki might change its behavior, you might think about
making the assertion that tests print_wiki’s output string somewhat
loose… just verify that some expected substring occurs in there
somewhere, rather than verifying that it is exactly equal to some
expected string.

Yup, that makes sense.

$0 is also going to present you some problems in a test… it will not
be the same value in the test as it is when run from your main
program. Not sure what to suggest about that without more context.

Better to supply the base name of the script as an argument?

3 other points:

looks like both methods have an unintended extra trailing + on their
first line

No??? On my screen they end with + “.wiki”.

standard style is to use 2 spaces for indentation; you appear to be
using 8

I do use a tab space of 2, however, this is the result of pasting from
vi and then tabs are expanded wildly (I have no solution for this).

since the 2 methods are nearly identical, you should pull out the
common code.

Sure.

Cheers,

Martin

@Brian

I have been looking at flexmock, but I guess the mocking method is not
important - it is how?!? … Especially since these are private methods.
Perhaps the code should be rearranged?

Cheers,

Martin

Brian C. wrote:

Martin H. wrote:

Ideas most welcome!

class Foo
def bar
system(“wibble”)
end
end

require ‘test/unit’
require ‘mocha’
class TestFoo < Test::Unit::TestCase
def test_foo
a = Foo.new
Foo.any_instance.expects(:system).with(“wibble”).returns(“bibble”)
assert_equal “bibble”, a.bar
end
end

Martin H. wrote:

Ideas most welcome!

class Foo
def bar
system(“wibble”)
end
end

require ‘test/unit’
require ‘mocha’
class TestFoo < Test::Unit::TestCase
def test_foo
a = Foo.new
Foo.any_instance.expects(:system).with(“wibble”).returns(“bibble”)
assert_equal “bibble”, a.bar
end
end

On 5/11/10, Martin H. [email protected] wrote:

@Caleb

I would extract a helper from both methods which does the bulk of the
work (the first 2 lines) but does not exit. Then test the helper and
don’t bother testing the methods that call it.

Hm, I thought all this unit testing was about testing the interface -
these are private methods. But perhaps this is one of those rare cases
where you have to do all sorts of tricks? I was wondering if a special
class for printing usage would be in order?

“Testing the interface” means different things to different people, I
guess. I think it is fairly obvious that the method below does not
need any tests, it’s clear to see that it’s correct just by looking at
it:

def foo_and_exit
foo
exit
end

I’m assuming that foo itself is tested, of course.

Yes, foo may not be the direct interface as seen by callers, but it is
very very close to it. Good enough for government work, as a former
colleague used to say.

I encourage you not to use mocks as others in this thread are
suggesting. Mocks divorce your tests from reality and force you to
test the implementation rather than the interface. The fact that you
program’s usage feature invokes the print_wiki command is surely an
internal detail, not an essential part of its interface, yes? Binding
your test to that internal detail is a greater sin than skipping the
test of exit in my opinion.

Mocks are necessary when testing externalities like databases and
network servers which are hard to set up or may not be available when
tests are run. So, ask yourself this question: is it reasonable to
expect that print_wiki will always be available when your tests are
run? Is it something that will already be installed or be fairly easy
to install and set up?

You then have the problem that this print_wiki command is printing
something to stdout, which you want to test. Either capture stdout in
the test and verify it contains what you want, or use backticks
(...) instead of system in your helper method.

Actually, ‘print_wiki’ outputs to stderr - which makes sense if you are
executing a command and piping the output to a file. You want to see the
usage info as an indicator that your did something wrong (if you did
mess up), and not have the usage information in the output file!

All right, stderr then. Same problem really. Except backticks won’t
work for you.

There are some existing utilities out there for capturing stderr and
stdout, but I’m not sure what to suggest. It’s also pretty easy to
write your own.

program. Not sure what to suggest about that without more context.

Better to supply the base name of the script as an argument?

Yeah, probably.

3 other points:

looks like both methods have an unintended extra trailing + on their
first line

No??? On my screen they end with + “.wiki”.

Ah, pastie hid their horizonal scroll bar way down at the bottom where
I couldn’t see it. My mistake. ( Your love of excessive indentation
isn’t helping here. :wink:

standard style is to use 2 spaces for indentation; you appear to be
using 8

I do use a tab space of 2, however, this is the result of pasting from
vi and then tabs are expanded wildly (I have no solution for this).

I suggest that you not use tabs. Use 2 spaces.

Martin H. wrote:

@Brian

I have been looking at flexmock

I used to use flexmock, but switched to mocha. The error messages were
much more informative, especially in terms of expectations not met or
unexpected invocations.

but I guess the mocking method is not
important - it is how?!? … Especially since these are private methods.

Sorry, I don’t understand you’re trying to say there.

‘system’ is a private method inherited from Kernel, but you can happily
mock or stub it within a test.

If you know the appropriate object instance, then

obj.expects(:system)…

is fine. If you don’t, but you know what class it is, then

Foo.any_instance.expects(:system)…

Unfortunately, Object.any_instance. doesn’t do the trick.

Perhaps the code should be rearranged?

That’s another option - for example, make print_wiki a method, which you
can mock out.

Or you could check for the required side-effects of print_wiki, by
running the test inside IO.popen, and checking the text written. This
will be slower, because you’re actually running the external print_wiki
command.

class Foo
def bar
system(“cat /etc/motd”)
end
end

require ‘test/unit’
require ‘mocha’
class TestFoo < Test::Unit::TestCase
def test_foo
IO.popen("-") do |child|
if child
assert_match /Linux/, child.read
else
a = Foo.new
a.bar
end
end
end
end

OK, I am getting closer, but are still a bit confused. I feel like
staying away from mocks, because I am not proficient enough to tell, if
my code and tests are working correctly or not.

Now, I have rearranged the code a bit → http://pastie.org/956728

I still have a problem with $0. I was wondering if I could just add the
script name to the parse method as an optional argument (or is that
ugly?):

def parse(argv, script_name=$0)

This would make it possible to write the following tests:

test “Options.parse with empty argv and missing wiki file raises” do
opt_parser = Options.new
assert_raise() { opt_parser.parse([],“foo”) }
end

test “Options.parse with empty argv and existing wiki file don’t raise”
do
opt_parser = Options.new
assert_nothing_raised() { opt_parser.parse([],“/foo/bar.rb”) }
end

However, the original problem remains - I really want to test the
following:

test “Options.parse with empty argv and existing wiki output short
usage” do
opt_parser = Options.new
assert(“” == opt_parser.parse([],“/foo/bar.rb”))
end

test “Options.parse with --help in argv and existing wiki output long
usage” do
opt_parser = Options.new
assert(“” == opt_parser.parse([“–help”],“/foo/bar.rb”))
end

I am actually not interested in the invocation of print_wiki, since this
Perl script is tested - so never mind about testing the
print_usage_and_exit → good enuf for government work… But I am
interested in testing the print_usage_full? and print_usage_short?
methods. So how to do that? There is this send trick for testing private
methods:

Martin

AFAICS this will always raise, since $? will never be nil. I think you
want:

raise ... unless $?.success?

Aaah, nice trick. I did notice that this always was raising, but I was
postponing forensics until RTFM :o)

So with a change of code (http://pastie.org/956910), including an
explicit file check, I now have this working:

test “Options.parse with empty argv and missing wiki file raises” do
opt_parser = Options.new
assert_raise() { opt_parser.parse([], “foo”) }
end

Well, that’s what mocking is for. You say you don’t actually want to run
print_usage_and_exit, so I think you want to check that it would have
been run, with the correct arguments to do what you want.

opt_parser = Options.new
opt_parser.expects(:print_usage_and_exit).with()
assert_nothing_raised { opt_parser.parse([], “/foo/bar.rb”) }

Right, so this appears to be working nicely:

test “Options.parse with empty argv and existing wiki file don’t
raise” do
opt_parser = Options.new
opt_parser.expects(:print_usage_and_exit).with()
assert_nothing_raised { opt_parser.parse([], SCRIPT_PATH) }
end

At the end of your test method, if :print_usage_and_exit wasn’t called,
or was called with different arguments, you’ll get an error raised by
the mocking framework. The awkward thing is that print_usage_and_exit
might actually be called multiple times, so it may be worth changing the
call to

if print_usage_full?
print_usage_and_exit(true)
return # not reached except in testing
end

This I don’t understand. How can the method be called multiple times,
and how can that be fixed by introducing a return? I do get this funky
error:

“- expected exactly once, already invoked once:
#Options:0x100958ef8.print_usage_and_exit(:full)”

from this (the case where print_usage_and_exit should be invoked with an
arg):

test “Options.parse with --help in argv and existing wiki output long
usage” do
opt_parser = Options.new
opt_parser.expects(:print_usage_and_exit).with(:full)
assert( “” == opt_parser.parse([“–help”],SCRIPT_PATH) )
end

and I don’t know what to assert.

As for $0: this tends to be dangerous anyway (it is subject to
truncation under various conditions). FILE is safer, giving the
location of the current source file, if that’s usable.

Yup, $0 is now gone.

Cheers,

Martin

Martin H. wrote:

test “Options.parse with empty argv and existing wiki file don’t raise”
do
opt_parser = Options.new
assert_nothing_raised() { opt_parser.parse([],"/foo/bar.rb") }
end

if full
  system("print_wiki --data_in #{@wiki_path} --help")
else
  system("print_wiki --data_in #{@wiki_path}")
end

raise "Failed printing wiki: #{@wiki_path}" if $?

AFAICS this will always raise, since $? will never be nil. I think you
want:

raise ... unless $?.success?

However, the original problem remains - I really want to test the
following:

test “Options.parse with empty argv and existing wiki output short
usage” do
opt_parser = Options.new
assert("" == opt_parser.parse([],"/foo/bar.rb"))
end

test “Options.parse with --help in argv and existing wiki output long
usage” do
opt_parser = Options.new
assert("" == opt_parser.parse(["–help"],"/foo/bar.rb"))
end

In that case you would need to collect the output from print_wiki, for
example using the IO.popen code I showed before.

I am actually not interested in the invocation of print_wiki, since this
Perl script is tested - so never mind about testing the
print_usage_and_exit -> good enuf for government work… But I am
interested in testing the print_usage_full? and print_usage_short?
methods. So how to do that?

Well, that’s what mocking is for. You say you don’t actually want to run
print_usage_and_exit, so I think you want to check that it would have
been run, with the correct arguments to do what you want.

opt_parser = Options.new
opt_parser.expects(:print_usage_and_exit).with()
assert_nothing_raised { opt_parser.parse([], “/foo/bar.rb”) }

At the end of your test method, if :print_usage_and_exit wasn’t called,
or was called with different arguments, you’ll get an error raised by
the mocking framework. The awkward thing is that print_usage_and_exit
might actually be called multiple times, so it may be worth changing the
call to

if print_usage_full?
print_usage_and_exit(true)
return # not reached except in testing
end

As for $0: this tends to be dangerous anyway (it is subject to
truncation under various conditions). FILE is safer, giving the
location of the current source file, if that’s usable.

But otherwise I would be inclined to move this logic out into the code
which calls the option parser: i.e.

def parse(argv, wiki_path)
@wiki_path = wiki_path

end

and then in your main bin file:

opt_parser.parse(ARGV, ENV[“BP_DIR”] + “/bp_usage/” +
File.basename($0, “.rb”) + “.wiki”))

Just a suggestion.

So with code:

if print_usage_full?
  print_usage_and_exit(true)
  return # break for unit testing.
elsif print_usage_short?
  print_usage_and_exit
  return # break for unit testing.
end

and test:

test “Options.parse with --help in argv and existing wiki output long
usage” do
opt_parser = Options.new
opt_parser.expects(:print_usage_and_exit).with(:full)
assert( “” == opt_parser.parse(["–help"],SCRIPT_PATH) )
end

I now get the following err:

optparse_swapcase_seq_test.rb:183:in `block in class:OptionTest’]:
unexpected invocation: #Options:0x100953a70.print_usage_and_exit(true)
unsatisfied expectations:

:o/

and I am still not sure what exactly to assert …

M

Brian C. wrote:

Martin H. wrote:

so it may be worth changing the
call to

if print_usage_full?
print_usage_and_exit(true)
return # not reached except in testing
end

This I don’t understand. How can the method be called multiple times,
and how can that be fixed by introducing a return? I do get this funky
error:

“- expected exactly once, already invoked once:
#Options:0x100958ef8.print_usage_and_exit(:full)”

Your code is:

...
print_usage_and_exit(true) if print_usage_full?
print_usage_and_exit       if print_usage_short?

options_default
...

When running normally, if you call print_usage_and_exit(true) then
inside it calls ‘exit’ which terminates the program.

But if you mock out print_usage_and_exit then it will return, and
continue to the next line. So if print_usage_short? is also true, it
will call print_usage_and_exit() as well.

I think that’s what the error message is telling you. By default the
expectation set is that the method will be called exactly once, and it
was actually called more than once. (Mocha lets you override this if you
want it to be called more than once)

I should add that I’m actually not a big fan of mocking; working out how
to get the mocking framework to do what I want distracts me from actual
programming. But in this case, it looks like the right tool for the job.

he error is pretty clear when you break it down.

optparse_swapcase_seq_test.rb:183:in `block in class:OptionTest’]:
unexpected invocation: #Options:0x100953a70.print_usage_and_exit(true)

“You called print_usage_and_exit(true), and I wasn’t expecting that”

unsatisfied expectations:

“I was expecting you to call print_usage_and_exit(:full), and you never
did”

So the expectation you set (that the method would be called with
argument ‘:full’) doesn’t match the reality (that your code calls the
method with argument ‘true’)

Martin H. wrote:

so it may be worth changing the
call to

if print_usage_full?
print_usage_and_exit(true)
return # not reached except in testing
end

This I don’t understand. How can the method be called multiple times,
and how can that be fixed by introducing a return? I do get this funky
error:

“- expected exactly once, already invoked once:
#Options:0x100958ef8.print_usage_and_exit(:full)”

Your code is:

...
print_usage_and_exit(true) if print_usage_full?
print_usage_and_exit       if print_usage_short?

options_default
...

When running normally, if you call print_usage_and_exit(true) then
inside it calls ‘exit’ which terminates the program.

But if you mock out print_usage_and_exit then it will return, and
continue to the next line. So if print_usage_short? is also true, it
will call print_usage_and_exit() as well.

I think that’s what the error message is telling you. By default the
expectation set is that the method will be called exactly once, and it
was actually called more than once. (Mocha lets you override this if you
want it to be called more than once)

I should add that I’m actually not a big fan of mocking; working out how
to get the mocking framework to do what I want distracts me from actual
programming. But in this case, it looks like the right tool for the job.

Aha! Great!

This seems to work great:

test “Options.parse with --help in argv and existing wiki output long
usage” do
opt_parser = Options.new
opt_parser.expects(:print_usage_and_exit).with(true)
assert_nil( opt_parser.parse(["–help"],SCRIPT_PATH) )
end

(Probably the assertion should be something more sane)

Thanks Brian, much appreciated!

Martin

Brian C. wrote:

he error is pretty clear when you break it down.

optparse_swapcase_seq_test.rb:183:in `block in class:OptionTest’]:
unexpected invocation: #Options:0x100953a70.print_usage_and_exit(true)

“You called print_usage_and_exit(true), and I wasn’t expecting that”

unsatisfied expectations:

“I was expecting you to call print_usage_and_exit(:full), and you never
did”

So the expectation you set (that the method would be called with
argument ‘:full’) doesn’t match the reality (that your code calls the
method with argument ‘true’)

Martin H. wrote:

(Probably the assertion should be something more sane)

There’s no need to assert anything - the mock setup is adding assertions
at the end of the test, as you already saw.

If you do want to assert explicitly that everything went normally, then
assert_nothing_raised {} is what I would use.

Thanks Brian, much appreciated!

You’re welcome.