small medium large xlarge

Ironman_pragsmall
19 May 2016, 15:23
Steve Hunter (2 posts)

At page 51 of the printed book, we create a series of tests to check that the written code can cope with a single string, a single string with size, multiple tasks etc.

To accomplish this, we need to split up the string entered in the text area by the user. A method called convert_string_to_tasks is written for this purpose. That iterates over each \n separated line, and then further breaks the task down with a : delimiter too.

My concern is that if the functionality is being added to the test which isn’t replicated in the application code. Take the test for a single string:

    it "handles a single string" do
    	creator = CreatesProject.new(name: "Test", task_string: "Start things")
    	tasks = creator.convert_string_to_tasks
    	expect(tasks.size).to eq(1)
    	expect(tasks.map(&:title)).to eq(["Start things"])
    	expect(tasks.map(&:size)).to eq([1])
    end

This test passes even though the code behind doesn’t contain that functionality yet:

def build
    self.project = Project.new(name: name)
    # project.tasks = convert_string_to_tasks
    # project
  end

With the two lines commented out (where this functionlty is delivered) the test passes as the test itself contains the functionality held within the commented out code. The test calls the convert_string_to_tasks method, so that the code doen’t need to. The test isn’t checking the code functions correctly; it is checking that the code could function correctly if properly implemented, which it is not.

This is the same across all except the last of the "task string parsing" tests. Only the last test for "attaches tasks to the project" does the code need to do the heavy-lifting.

Is it not better to use the approach contained in the last test across all the tests such that the functionality is held in the code not the test, resulting in something like:

it "handles a single string" do
  creator = CreatesProject.new(name: "Test", task_string: "Start things")
  creator.create # use the applied functionality
  expect(creator.project.tasks.size).to eq(0)
  expect(creator.project.tasks.map(&:title)).to eq(["Start things"])
  expect(creator.project.tasks.map(&:size)).to eq([1])
end

That tests that the code works without the test holding the logic/workflow but adds some ugly repetition that needs refactoring! Am I missing something?

Steve.

Head_shot_pragsmall
14 Sep 2016, 19:37
Noel Rappin (48 posts)

Thanks for the well-thought out comment, I’m sorry I haven’t gotten to it sooner.

The earlier tests are testing that the convert_string_to_tasks method works properly, you categorize that as “checking that the code could function correctly”, I’d be more likely to characterize it as “checking that the particular method under tests works correctly.

The last test using create is more of a small-scale integration test, testing both that the unit works and that the unit acts correctly (some people would advocate using a test double there to stub out convert_string_to_tasks).

Functionally, you absolutely could write all the tests to call create, and in an example this small it wouldn’t make any difference. Though those tests would be testing both create and convert_string_to_tasks, making them a bit more fragile. Again, not much of a practical difference in the example.

Noel

You must be logged in to comment