views:

612

answers:

12

What is the best way to unit test a method that calls into multiple methods, for example:

modify(string value)
{
    if(value.Length > 5)  replaceit(value);

    else changeit(value);
}

This pseudo code has a modify method that (currently) calls either replaceit() or changeit(). I have already wrote tests for replaceit and changeit, so writing a new test for modify will be 99% the same set of code. I need to test it thought because it may change in the future.

So do I copy paste the existing test code? Move the test code to a common function? Any other ideas? I'm not sure of the best practice here.

+4  A: 

If you've already tested replaceit() and changeit() independently, then the only thing you have left to test if the if condition. Test modify() with a few values to make sure it calls the right function under the right conditions (those conditions being null and Strings of length 4, 5, and 6 for the example code you gave).

Bill the Lizard
+2  A: 

Well, no, your test code isn't going to be 99% the same, because you're actually testing something different here, unless replaceit, changeit and modify all return the same values.

Not sure why the difficulty. The test for the modify method should be approximately four lines long. Since you're already testing the underlying functionality and all you want to do is make sure this particular method doesn't break, writing a test that tests the two possible code paths in this function return expected values should be sufficient.

DannySmurf
+1  A: 

What is "the test code" in this case? Setting up and checking the results? If so, I'd refactor it into a different method and use it from each of the tests. I'd only do this if there's a sigificant amount of it though - there's a readability benefit to being able to see everything that a test does, just by reading the code of that method.

Complicated test methods often bother me to start with, to be honest - often they can't realistically be avoided, but if you can simplify them, it's worth doing so.

Jon Skeet
I disagree with factoring out asserts from tests into helper functions. I think test code is much more readable/maintainable when all the asserts stay in the test functions. When people write factor out asserts, they tend to start "shotgun testing", and stop thinking about their actual test cases.
Scotty Allen
@Scotty: It depends. Some tests are just unavoidably longwinded to set up and check. We can try to avoid it all we like, but sometimes it's just necessary. Helper methods can save a lot of repetitive code.
Jon Skeet
+1  A: 

If you already wrote tests for replaceit() and changeit() the test for modify would simply check that different results are returned depending on the value of 'value'. However you will simply be reimplementing the logic of the method in the test, which is a little absurd.

In this case I wouldn't test modify until it has more complex logic, or better - is used by another method that is more significant to test.

Eran Galperin
+1  A: 

Just test modify.

Modify is supposed to return certain values when given certain values.

It's unimportant how modify does its job - only that it does its job.

And if, in the future, you change modify to use different methods (or no methods), it does not, and should not, and will not, affect your tests.

That said, also test replaceit' and changeit`.

Ian Boyd
+1  A: 

You basically need 2 tests.

1) Pass in a string like "The Quick Brown Fox Jumps!" (length greater than five) makes sure that the value is affected by replaceit(...)

2) Pass in a string like "Foo" (length is less than five) and make sure that the value is affected by changeit(...)

Your test (in pseudo code) might look like this:

testLongValue() {
    string testValue = "A value longer than 5 chars";
    string expected = "Replaced!";
    string actual = modify(testValue);
    assertEqual(expected, actual);
}

testShortValue() {
    string testValue = "len4";
    string expected = "Changed!";
    string actual = modify(testValue);
    assertEqual(expected, actual);
}

Obviously I could give you a more realistic example if I knew what replacit() and changeit() were supposed to do, but this should give you the idea. If it mutates the original value reference instead of returning it, you can just use testValue as the actual value after the call occurs.

Justin Standard
This is how I would do it as well. Since you've tested replaceit() and changeit() fully on their own, you only need to test the logic that's contained in modify(). A test case that triggers replaceit(), and one that tests changeit(), both checking to see they did what was expected, should be fine.
Scotty Allen
I'd add the limit case where the length is exactly 5
Kena
I'd also add a test for null, just as a reminder to myself to add a guard clause.
Bill the Lizard
+2  A: 

When testing boundary conditions like if (value.length > 5) you should make sure your test data contains values of value that have length 4, 5, or 6.

bmatthews68
+8  A: 

This is a classic state-based test vs. behavior-based test scenario.

In this ridiculously simple example testing the output is fine. At some point though, you'll run into tests where inspecting the state after execution is complicated. Instead you want to check the behavior (e.g. verify that changeit was called with a specific value).

At that point you probably should look into a mock object framework like Rhino.Mocks (.Net) or Mockito (Java) and start writing more interface based code.

Eric Nicholson
A: 

Same as Justin Standard, plus passing null as value (which obviously will fails for the code snippet you give us ;)) Basic rule for Unit testing is "test only what is specific to the method under test". And it quite ... uncommon to have a method that doesn't call another one.

Olivier
+3  A: 

You have a number of options. Which one is best depends on details that aren't clear from your question.

  • test modify just as if it were an unrelated method. Advantage: it might at some point become one.
  • just test that you got the if-statement right. That is, just test enough that the tests force you to write the implementation you need (where calling replaceit and changeit is just the simplest implementation that could possibly work. If your are practicing TDD, this should come naturally to you. Advantage: high test coverage without much duplicated effort.
  • Subclass and Override Method (this is a dependency breaking technique from the book "Working Effectively With Legacy Code"): Test the method on a subclass that you introduce solely for testing purposes, which overrides replaceit and changeit with canned answers or so that they set sensing variables (variables that indicate whether the method has been called with the right value(s)). Advantage: might possibly simplify your tests (or not), sometimes even just make testing possible.
  • Extract a new class for the replaceit and changeit methods, including an interface for that class. Stub or Mock that interface when testing modify. Advantage: might both make your design more testable and better decoupled/reusable in general (or not).
Ilja Preuß
+1 Option 2 - Option 3 - Option 4 in that order of preference.
Gishu
+1  A: 

Maybe this will help... Unit Testing Methods that Call Methods

Brandon Joyce
+2  A: 

In order of preference

  1. modify(test) just has 2 scenarios (each arm of the if stmt), so I'd write 2 tests for modify of the form.
    If expected result of replaceit(value) is easy to determine..

.

public TestModifyIfValueLength..()
    {
      string expectedValue = .. ;// literal result of replaceit(value)
      Assert.Equals( expectedValue, modify("asd") );
    }
  1. If not, consider using a stub (use subclass and override changeit, replaceit) to verify that the correct method was called.
  2. If the stub is too much work, do the Mock thing. Extract an interface and setup expectations on changeit, replaceit.

Assumptions

  • You have tests for replaceit(value) and changeit(value), which test (e.g. all boundary conditions for) those 2 methods comprehensively.
  • replaceit() and changeit() are public methods.. If not, you should consider writing tests against the public methods only. You should be free to tweak / chuck out private methods without the test code knowing.
Gishu