Is it okay to run for loops in functional test methods?

Question

Is it okay (conceptually) to run for loops in test methods?

I'd like to test a range of parameter values into a controller, to determine if the different inputs return the correct values.

  test "logged in user - add something - 0 qty" do
    @app = Factory.create(:app)

    (0..5).each do |t|
      @qty = t
      login(:user)
      get :add. :id => @app.id, :qty => @qty
      assert_nil(flash[:error])
      assert_response :redirect
      assert_redirect_to :controller => :apps, :action => :show, :id => @app.id
      if @qty = 0 
        assert_equal(Invite.all.count, @qty + 1)
      else 
        assert_something .........
    end
  end

Something like that.

Answer 1

Is it okay (conceptually) to run for loops in test methods?

You mean, is it politically correct?

It works, right? I'm trying to imagine what the objection would be.

Answer 2

I usually try to avoid any kind of conditional statements or loops in test code. You want your tests to be as simple as possible, and if you start including logic in your tests you have to test them to make sure they work as designed. I would break the loop up into separate test cases, that way if any one of them fails it's easier to pinpoint exactly what inputs caused the failure. When a test fails it should be immediately obvious what caused it. You shouldn't have to analyze the test code to figure it out.

Update:

I do want to add that there are some extremely rare cases where you would want to have a loop in your test cases. One specific example is when you're testing for concurrency issues. This is an exception to the general rule, and you should have a very good and well-understood reason for having any kind of logic in your tests.

Answer 3

It is even better if you can have your framework be aware that this one single test is actually performing multiple tests (with different parameters). It allows you to see what exact parameter combination fails and which succeed in the test-report.

Answer 4

Tests should also be viewed as "living documentation" of what your software is supposed to do, so keep them as clear as possible.

Answer 5

It is not politically correct to use more than one assertion or validation in each test. That said, everyone does it. One of the new styles is seen in Cucumber's test scenarios where the scenario is still in an extremely readable format but allows multiple sets of data to be tested.

But it's Ruby, you wouldn't be using it if you were the sort to follow everyone else's guidance to the letter. There is no right way, only the most common and that changes quite often.

I once asked my dentist which order I should brush, floss, and rinse. He told me that he didn't care if I managed to actually do all three. I believe the point was that oftentimes substandard implementations are better than none at all. If loops make testing more fun and therefore more likely then you should loop the holy hell out of your tests.

Answer 6

Hey.
There may be cases where you need loops but yours is not necessary one of them. Remember that adding more complexity to tests makes harder to work with them. When application evolves tests evolve too. If you make them too complex at start, than one day you may face a choice:

• should I spend 3 days to refactor this big old cluttered test that fails?
• should delete the test and write new, simpler elegant (in 3,5 day)?

This is hard choice. In first option you waste time for implementing new features for something that doesn't push project forward? Do you have time for this? Does your manager think you have time for this? Does client paying for your time on this project think that you have time for this?
Second option seams to be reasonable but, when writing new tests how do you know that you covered all cases as old one (plus new ones)? Is it all in documentation? Is it in test documentation? Do you remember all of them? Or maybe you go through test code and refactor it to reveal all cases hidden inside this code blob? Isn't this becoming first option?

Don't make tests like legacy code. Code no one wants to touch, no one really knows, everyone trying to avoid and ignore it as much as possible. Tests should be designed as rest application. Apply many design principles as you are applying to code design. Make them simple. Separate responsibility. Group them logically. Make them easy to refactor. Make them extensible. There are many things you should take into consideration.

As for your case. Let's Assume that you have use case where your code does something for parameter in <0,100> (0..5 from your code is close together, example looks more clear when wider range is used). In with other values it does some exception handling. In this case you want test cases:

• when parameter = -1
• when parameter = 0
• when parameter = 1
• when parameter = 99
• when parameter = 100
• when parameter = 101

Simple, separate test cases that are easy to refactor, easy to read while still checking code properly.
You could add test case where you would use loop to check behavior when parameter is in (10,70) but it is not recommended. With lots of tests and wide parameters ranges it is just waste of resources. If algorithm is deterministic, does the same steps for some set of values it will work for all of them if it works for one.
Try to read about equivalence classes, boundary values, pairwise testing, path coverage, statement coverage, branch coverage and other testing techniques to make your tests better.

Answer 7

I'm generally OK with a loop in a test as long as there's only one occurrence of an assertion in the test. In other words, this is OK:

test "something" do
  for item in @collection
    assert_something item
  end
end

But this is not:

test "something" do
  for item in @collection
    assert_something item
    assert_something_else item
  end
end

And these will make you hate yourself:

test "something" do
  for item in @collection
    assert_something item
    if x == y
      assert_something_else item
    end
  end
end

test "something" do
  for item in @collection
    assert_something item
  end
  for item in @collection
    assert_something_else item
  end
end

And the only time I would write a test this way is if the items in the collection vary from each other substantially, but there was some need to verify commonly shared behavior. For instance, you might have ten instances of different objects, but they all are supposed to respond to some message. So you verify that all instances can do the thing that the duck-typed method is supposed to do. But if you have a collection that always contains instances of Foo, you're often better off just making an assertion about @collection.first. The test executes faster, and you don't really gain much from repeating the assertion on all instances, and in most cases, you're much better off just testing item in isolation from the rest of the collection anyway. If you're feeling particularly paranoid, this is generally OK:

test "items are all Foo" do
  for item in @collection
    assert_kind_of Foo, item, "Everything in @collection must be Foo."
  end
end
test "something" do
  assert_something @collection.first
end

The "something" test will fail anyways if you've got non-Foo objects in the collection, but the preceding test makes it abundantly clear what the real problem is.

In a nutshell, avoid it, but if you've got a good reason to do it, then go ahead. And if it becomes a problem down the road, the test should still be simple enough that refactoring it into something less problematic is easy.

Answer 8

I'm going to add myself to the list of the politically incorrect. When testing a range of values, a loop can increase the readability of a test. Furthermore, it aids DRY, making refactoring easier: would you rather add the new parameter to the eight places the method is called in the test, or to just one?

Here's a test that uses this technique. It's using a home-grown test library, but the technique is universal:

  def test_swap_name
    test_cases = [
      [
        'Paul, Ron P.A.',
        'Ron Paul PA'
      ],
      [
        "PUBLIC, SR., JOHN Q",
        "JOHN Q PUBLIC SR"
      ],
      [
        "SMITH, JR., MARK A",
        "MARK A SMITH JR"
      ],
      [
        'James Brown',
        'James Brown'
      ],
      # (more test cases)
    ]
    for original, swapped in test_cases
      assertInfo("For original = #{original.inspect}") do
        assertEquals(original.swap_name, swapped)
      end
    end
  end

assertInfo adds an arbitrary string to the beginning of any exception message. That's how you can know, when a test failed, which data was being tested:

./StringUtil.test.rb
Method "test_swap_name" failed:
Assert::BlownAssert: For original = "Paul, Ron P.A.": Expected:
"Ron Paul PA"
but got
"Paul, Ron P.A."
./../../testlib/Assert.rb:125:in `fail_test'
./../../testlib/Assert.rb:43:in `assertEquals'
./StringUtil.test.rb:627:in `test_swap_name'