Do large test methods indicate a code smell?

Question

I have a particular method called TranslateValues() (Cyclomatic-Complexity of 5) which I would like to test.

The test requires a substantial number of mock objects which take up most of the method; The method being tested is fairly straightforward with this exceptional requirement. I am suspicious of this. For me, I might do something like this to alleviate the issue:

namespace TestArea
{
    [TestClass]
    public class PrimaryTests
    {
        //commonly used classes should go up here
        private CoreServiceClient client;
        public CoreServiceClient Client
        {
            get { return client; }
            set { client = value; }
        }

        [TestMethod]
        public void TestMethodForXYZ()
        {
            try
            {
              //use the Client, do some stuff, and assert
            }
            catch
            {
             //catch any problems, report them, and assert.fail
            }
        }
   }
}

But this will only take you so far in reducing the size and clutter of test methods, and so it begs the question: Are big test methods generally indicative of code smell? How do you gauge when your methods are becoming too big?

Answer 1

I say yes. You are testing too much.

I'm guilty of it myself but ideally you test one thing at a time (to be read as "one assert per test"). That's the Utopian view and so long as you're closer to that than the "one test for everything with a ton of asserts" side you're ok.

I think it really comes down to properly using the tool. NUnit, for example, has TestCaseAttributes so that you can have one test that runs through several cases, which you've basically described. Running them all through one test is less useful because it should stop at the first failure so you can only tell if you broke multiple cases by fixing the individual failures as you see them.

As for gauging when they are too big, it's pretty much as soon as I want to have a specific Assert run (and no others) but I can't because the test must run several other Asserts to remain valid.

In the end it really means following Single Responsibility when writing tests as well as code.

Answer 2

Why your test could be long:

• you are testing more than one thing
• sut dependencies require lot of mocking (probably your case)
• your sut requires lot of dependencies

If you are testing more than one thing

Write separate tests, for testing each thing. Each test will require less arranging, and less assertions. When some test will be broken, you will know what exactly is not working as expected.

If your mocks require lot of setup

First of all, you should verify why your sut asking it's dependency too much. Maybe you make decisions based on other object's state, thus violating Tell, Don't Ask principle. Consider moving that decisions logic to dependency.

// for this code you should setup mocks of person and address
if (person.Address != null) {
  Address address = person.Address;
  address.City = city;
}

vs

person.RelocateTo(city);

If you completely sure, that all logic in it's place, but your mock still requires some big setup, then consider moving mock creation to separate method. This also will allow to reuse that method in several tests.

Mock<Foo> fooMock = new Mock<Foo>();
fooMock.Setup(foo => foo.X).Returns("x");
fooMock.Setup(foo => foo.Y).Returns("y");
fooMock.Setup(foo => foo.Z).Returns("z");
Bar bar = new Bar(fooMock.Object);

vs

Foo foo = CreateFoo("x", "y", "z");
Bar bar = new Bar(foo);

If your sut requires lot of dependencies

If some of your dependencies always used together, than consider to introduce one object, which will hold them. Also check do you really need whole object to be passed. Maybe you just need one field from that object.

public void Foo(TimeProvider timeProvider)
{
   DateTime time = timeProvider.GetCurrentTime();
   ...
}

vs

public void Foo(DateTime currentTime)
{
   ...
}

Answer 3

I don't believe that a big test method is code smell by itself.

However, there are signs, for example when you

• need to mock too much dependencies (too much is relative too)
• need to setup too many things
• need to know too many things about the actual implementation detail (when you do TDD it won't be a problem)
• can't separate the testing of a method to well definied test cases (different test methods)

From my experience, one of the biggest sign that a method doing to much is when you can't really write a specific test case, just something like TestOfMyMethod(). That is probably not a test case, just a big mash of code covering the exact implementation of the actual method, which will break the next time you change anything.

Answer 4

No, not always. Big test methods are often results of testing/mocking frameworks and their workings (or, precisely - how you're supposed to use them).

For example, method might be perfectly fine and well-designed, but let's imagine it uses 2 or 3 dependencies - in order to test it, you'll probably have to stub/mock those dependencies. This adds extra lines of code to test method, which don't feel like attributing to tested method at all.

Having said that, too much dependencies might be good indicator (which, also will result in longer test method code due to extra setup work) and that's where I'd stay cautious, not the length itself.

---

Very trivial example of fairly common test, where test method will be "long", as compared to what it's actually testing (which probably is 1 line of code):

[Test]
public void GenerateReport_ThrowsInvalidOperationException_WhenTaskIsNotReady()
{
    var providerMock = new Mock<ITaskProvider>();
    providerMock.Setup(m => m.IsTaskReady).Returns(false);
    var sut = new ReportGenerator(providerMock);

    Assert.That(sut, Throws.InstanceOf<InvalidOperationException>());
}

Now, add bit more complex .Setup call, bit more complex assertion - this results in even more extended size of test method.