Red Green Repeat Adventures of a Spec Driven Junkie

TDD In-depth Divide - Part 2

Following from my previous article, TDD In-depth Divide Part 1, this article moves forward with more tests and code!

New York Daily News - William Michael Harnett

Requirements

Before starting, I like to have a consistent environment that is working. The environment I will be using is from a Vagrant box. The set up files are here:

https://github.com/a-leung/vagrant_files/tree/master/ruby_rspec_development

Installing Vagrant and Virtualbox information can be found at:

With the file downloaded, run: $ vagrant up, which will create a working environment. Connect to the environment using: $ vagrant ssh.

All of the commands and code will be working from within the vagrant environment.

I will continue exactly where the last article ends.

Test: Add Arguments

Divide method without any arguments is not very useful. So I will add two arguments to the method: a numerator & denominator, which follows normal division convention:

numerator / denominator

To create this test, I will follow a similar pattern from a pervious test, namely: expect { divide }.to_not raise_error.

spec/divide_spec.rb contents:

  it 'accepts two arguments - numerator & denominator' do
	expect { divide(1,1) }.to_not raise_error
  end

The result when running the test:

vagrant@ubuntu-xenial:~/tdd_series$ rspec
..F

Failures:

  1) arguments accepts two arguments - numerator & denominator
     Failure/Error: expect { divide(1,1) }.to_not raise_error
     
       expected no Exception, got #<ArgumentError: wrong number of arguments (given 2, expected 0)> with backtrace:
         # ./divide.rb:1:in `divide'
         # ./spec/divide_spec.rb:16:in `block (3 levels) in <top (required)>'
         # ./spec/divide_spec.rb:16:in `block (2 levels) in <top (required)>'
     # ./spec/divide_spec.rb:16:in `block (2 levels) in <top (required)>'

Finished in 0.0189 seconds (files took 0.10238 seconds to load)
3 examples, 1 failure

Failed examples:

rspec ./spec/divide_spec.rb:15 # arguments accepts two arguments - numerator & denominator

As expected, the method as is does not have any arguments.

Code: Add Arguments

This is a easy change to make that test pass: add arguments to the method definition.

divide.rb contents:

def divide(numerator, denominator)
end

The result when we run the test?

vagrant@ubuntu-xenial:~/tdd_series$ rspec
.F.

Failures:

  1) setup has divide function defined
	 Failure/Error: expect { divide }.to_not raise_error

	   expected no Exception, got #<ArgumentError: wrong number of arguments (given 0, expected 2)> with backtrace:
		 # ./divide.rb:1:in `divide'
		 # ./spec/divide_spec.rb:10:in `block (3 levels) in <top (required)>'
		 # ./spec/divide_spec.rb:10:in `block (2 levels) in <top (required)>'
	 # ./spec/divide_spec.rb:10:in `block (2 levels) in <top (required)>'

Finished in 0.0191 seconds (files took 0.10225 seconds to load)
3 examples, 1 failure

Failed examples:

rspec ./spec/divide_spec.rb:9 # setup has divide function defined

Now the new test passes, but the original tests that tests for the method definition fails, because its test set up is:

expect { divide }.to_not raise_error

where the new test set up is:

expect { divide(1,1) }.to_not raise_error

In the first article, there was a decision to follow the warning message of testing for the non-existence of a specific error, it is better to just to test for no errors. source.

Since it’s impossible to satisfy the two tests at the same time to have code such that there are no errors when there are: no arguments and two arguments.

Decision: Test Modification

Based on a previous test, the code is in a situation where there’s no reasonable solution to satisfy the tests as is. What can we do?

  • Option 1: remove original test
  • Option 2: skip new test
  • Option 3: keep both, adjust tests

Remove Original Test

The original test was written in a stage of development where there was less structure and this was a perfectly reasonable test to have.

This test may be deemed too fundamental, so removing it would allow flexibility for future tests.

At the same time, it’s fundamental aspect is a benefit, as this tests whether the method exists in general (i.e. were imports done properly.)

Remove New Test

The new test does everything the old test did and more. It will define how the method works for all future incarnations.

This test won’t cover when a fundamental aspect of the function changes, such as if the method was modified to handle more arguments.

Keep Both

Is there a way to keep both tests? Not if both tests has to exist in their current form.

The main reason not to keep both is the framework won’t be able to support the tests in the current form. They are in opposition.

If the tests are slightly adjusted, both can exist in the test suite.

The adjustment would be: make both tests more specific, to test the specific expectation: NameError and ArgumentError, respectively.

So, change the test:

spec/divide_spec.rb contents:

  it 'has divide function defined' do
	expect { divide }.to_not raise_error
  end

Will have to backtrack on this and revert the test to:

spec/divide_spec.rb contents:

  it 'has divide function defined' do
	expect { divide }.to_not raise_error NameError
  end

to test function existence.

and:

spec/divide_spec.rb contents:

describe 'arguments' do
  it 'accepts two arguments - numerator & denominator' do
	expect { divide(1,1) }.to_not raise_error 
  end
end

to:

spec/divide_spec.rb contents:

describe 'arguments' do
  it 'accepts two arguments - numerator & denominator' do
	expect { divide(1,1) }.to_not raise_error ArgumentError
  end
end

Running the tests again using $ rspec:

vagrant@ubuntu-xenial:~/tdd_series$ rspec
.WARNING: Using `expect { }.not_to raise_error(SpecificErrorClass)` risks false positives, since literally any other error would cause the expectation to pass, including those raised by Ruby (e.g. NoMethodError, NameError and ArgumentError), meaning the code you are intending to test may not even get reached. Instead consider using `expect { }.not_to raise_error` or `expect { }.to raise_error(DifferentSpecificErrorClass)`. This message can be suppressed by setting: `RSpec::Expectations.configuration.on_potential_false_positives = :nothing`. Called from /home/vagrant/tdd_series/spec/divide_spec.rb:9:in `block (2 levels) in <top (required)>'.
..

Finished in 0.0051 seconds (files took 0.11118 seconds to load)
3 examples, 0 failures

Test passes, but now the original warning message comes back.

As there are specific tests for each type, let’s turn off this warning by adding into

spec/divide_spec.rb contents:

RSpec::Expectations.configuration.on_potential_false_positives = :nothing

to the top of the file.

Ok, now that the divide method exists and can accept a numerator and denominator the next step is to…

Start Dividing!

Now the test suite handles a few edge cases, let’s start to divide! There’s been a lot of tests written, but nothing to the actual divide function!

Test: Divide by 1

We will start dividing by the most simple case: 1. Whatever is divided by 1 should be itself, so the test would be:

spec/divide_spec contents:

describe 'divides' do
  it 'by 1 properly' do
	expect(divide(1,1)).to eq(1)
  end
end

Running the test:

vagrant@ubuntu-xenial:~/tdd_series$ rspec
...F

Failures:

  1) divides by 1 properly
	 Failure/Error: expect(divide(1,1)).to eq(1)

	   expected: 1
			got: nil

	   (compared using ==)
	 # ./spec/divide_spec.rb:22:in `block (2 levels) in <top (required)>'

Finished in 0.01942 seconds (files took 0.10236 seconds to load)
4 examples, 1 failure

Failed examples:

rspec ./spec/divide_spec.rb:21 # divides by 1 properly

Yup, the current code for the divide method would not support this.

Code: Divide by 1

Let’s do the absolute minimum to cover this test:

divide.rb contents:

def divide(numerator, denominator)
  1
end
vagrant@ubuntu-xenial:~/tdd_series$ rspec
....

Finished in 0.00783 seconds (files took 0.10324 seconds to load)
4 examples, 0 failures

Yup, it’s super simple, but it gets the job done! Remember, I don’t want anymore code than necessary to pass the test. Anything more would be extravagant!

Test: Divide 2 by 1

Let’s step up with a test: divide 2 by 1.

Exciting, isn’t it??

spec/divide_spec.rb contents

describe 'divides' do
  it 'by 1 properly' do
	expect(divide(1,1)).to eq(1)
	expect(divide(2,1)).to eq(2)
  end
end

I put the new test within the existing it block. Why? It’s just a preference. When I have many small tests in a row, I just like to keep them together. Setting up a new it block is possible too.

vagrant@ubuntu-xenial:~/tdd_series$ rspec
...F

Failures:

  1) divides by 1 properly
	 Failure/Error: expect(divide(2,1)).to eq(2)

	   expected: 2
			got: 1

	   (compared using ==)
	 # ./spec/divide_spec.rb:23:in `block (2 levels) in <top (required)>'

Finished in 0.02438 seconds (files took 0.12145 seconds to load)
4 examples, 1 failure

Failed examples:

rspec ./spec/divide_spec.rb:21 # divides by 1 properly

By having simple code does have it’s disadvantage: every new case will have to be accounted for

Code: Divide 2 by 1

What’s the most minimal approach to solving the tests? Since the tests are only testing division by 1, returning the numerator would be sufficient:

divide.rb contents:

def divide(numerator, denominator)
  numerator
end
vagrant@ubuntu-xenial:~/tdd_series$ rspec
....

Finished in 0.00642 seconds (files took 0.10502 seconds to load)
4 examples, 0 failures

That should take care of things!

Test: Divide any value by 1

The last code would probably handle any additional divide by 1 tests that can be written. Instead of writing: divide 3 by 1, let’s write a test that divides any number by 1:

spec/divide_spec.rb contents:

describe 'divides' do
  it 'by 1 properly' do
	expect(divide(1,1)).to eq(1)
	expect(divide(2,1)).to eq(2)

	numerator = rand(1..1_000_000)
	expect(divide(numerator, 1)).to eq(numerator)
	end
end

This test is kind of nice since it covers a random value between a large range.

vagrant@ubuntu-xenial:~/tdd_series$ rspec
....

Finished in 0.00669 seconds (files took 0.10333 seconds to load)
4 examples, 0 failures

Alternative test

Another way to write this:

spec/divide_spec.rb contents:

describe 'divides' do
  it 'by 1 properly' do
	expect(divide(1,1)).to eq(1)
	expect(divide(2,1)).to eq(2)

	(1..1_000_000).each do |numerator|
		expect(divide(numerator, 1)).to eq(numerator)
	end
  end
end

This is a thorough test that exercises the method upto one million (or any value desired.)

I would write this if it was important to cover all the values each time to run the test suite.

Introducing randomness into a test suite has a bit of a drawback in that when tests fail randomly, it becomes hard to debug, so writing a test that tests exhaustively may be preferred.

With the base case is covered. Let’s move onto the next test. Can you guess what it is?

Test: Divide by 2

Yes, the logical test after dividing by 1: it’s divide by 2!

This is where people go nuts from TDD or start blogging about TDD. If you’re the latter and interested in a new role, contact me.

So, let’s start the test:

spec/divide_spec.rb contents:

  it 'by 2 properly' do
	expect(divide(1,2)).to eq(0)
  end

Hmm… One thing I am not sure about: what does Ruby do when dividing 1 by 2? I know it’s 0.5, but programming languages handle division differently due to the processor handling of numbers.

Let’s double check how it is in Ruby by going to irb:

vagrant@ubuntu-xenial:~/tdd_series$ irb
2.4.5 :001 > 1/2
 => 0

Ok, so in Ruby, 1/2 is 0, not 0.5. So the test written is correct.

vagrant@ubuntu-xenial:~/tdd_series$ rspec
....F

Failures:

  1) divides by 2 properly
	 Failure/Error: expect(divide(1,2)).to eq(0)

	   expected: 0
			got: 1

	   (compared using ==)
	 # ./spec/divide_spec.rb:30:in `block (2 levels) in <top (required)>'

Finished in 0.01955 seconds (files took 0.10085 seconds to load)
5 examples, 1 failure

Failed examples:

rspec ./spec/divide_spec.rb:29 # divides by 2 properly

Code: Divide 1 by 2

Hmm… solving this is a little trickier considering there are previous tests that cover dividing by 1.

Again, going with the least amount of code to make the new test pass while passing all the previous tests, I decided to solve using an if block:

divide.rb contents:

def divide(numerator, denominator)
  if denominator < 2
	numerator
  else
	0
  end
end

Running the tests:

vagrant@ubuntu-xenial:~/tdd_series$ rspec
.....

Finished in 0.00931 seconds (files took 0.10359 seconds to load)
5 examples, 0 failures

Yup, this solution is a hack and if I came across this code in a production environment, I would immediately run: git blame divide.rb, go to this person’s desk and ask them what is going on?!

If this person’s response is:

That was the most stable way to make the tests pass

I would commend them on a job well done and go back to my desk.

Conclusion

I presented how I expect tests and code to be written, while also solving certain decisions, such as revising previous tests when new ones are incongruent.

To me, tests are a codification of these decisions and they are there to maintain this information, however rudimentary they are.

The flow of small tests with the least amount of code to make them pass is exactly the process I expect of myself and others.

I will continue on with another article on this, where the code might actually be able to divide!