I have a ruby script that I would like to apply unit testing against but
Ive never actually created any previously and the documentation is not
bad but im struggling to apply it to my own code.
Could someone give a couple of examples of how unit testing might be
applied to the following code just to get me started on it.
include REXML
def fetch(uri_str, limit=10)
fail 'http redirect too deep' if limit.zero?
puts "Scraping: #{uri_str}"
response = Net::HTTP.get_response(URI.parse(uri_str))
case response
when Net::HTTPSuccess
response
when NetHTTPRedirection
fetch(response['location'], limit-1)
else
response.error!
end
end
I have a ruby script that I would like to apply unit testing against but
Ive never actually created any previously and the documentation is not
bad but im struggling to apply it to my own code.
Could someone give a couple of examples of how unit testing might be
applied to the following code just to get me started on it.
I don't have time to look at your code now, but maybe someone will
pitch in, or I'll look in the morning. I also had trouble getting into
unit testing so I eventually bought a book and made a good study of
it.
I can recommend the book. Even though it covers C#, it's mostly generic info:
I love the pragmatic programmer books. Every time I buy one I'm
impressed. My all-time favourite one is "The Pragmatic Programmer:
Journeyman to Master". I think all programmers should read that book.
Les
···
On 11/21/06, Chris Gallagher <cgallagher@gmail.com> wrote:
Hi,
I have a ruby script that I would like to apply unit testing against but
Ive never actually created any previously and the documentation is not
bad but im struggling to apply it to my own code.
Could someone give a couple of examples of how unit testing might be
applied to the following code just to get me started on it.
Zentest sounds pretty cool but there doesnt seem to be anything telling
people how to use it at the moment. how does one get zentest to scan
your code or how ever it does what it does?
This is how I'd do it. I don't know the details, so your actual code
will be different. This code was not run at all, so probably there are
many typos/errors.
You'd certainly choose another names than I've chosen.
First of all, if you want to test, it's good to split the code in
small chunks so that each does a little bit of work. Thus you can test
them separately. I tend to put the code inside some class as well...
you know, the OOP
While adding tests, you want to change the code as little as possible,
to not broke it. When you have the tests in place, you can safely
change stuff (=refactor).
The final code could be a bit different from what is written in the
mail, as the code was written before the text, and I was too lazy to
fix it
Now the steps to the code below:
1. minor corrections:
- at the end of the code 'end' is missing
- change |records| to |result| as you are iterating over Result(s)
- change each to collect as you seem to collect results (but I may be wrong)
2. move the whole each/collect block inside Result class as:
class Result < ...
def self.get_records
Result.find(:all).each do |result|
...
end
end
end
(you can omit the Result. before find)
We have a function that returns something for all Results
what remains at the bottom is
records = Result.get_records
3. The block doesn't return anything meaningful so:
- add just after class Result < ...:
BuildStatus = Struct.new(:module_name, :build_status_since,
:last_failure, :last_success, :build_number)
- add BuildStatus.new(module_name, build_status_since, last_failure,
last_success, build_number) at the end of the block (inside it)
4. Now we can add the first unit test at the very bottom:
comment out the records = ... line
and add:
if __FILE__ == $0
require 'test/unit'
class TestResult < Test::Unit::TestCase
def test_get_records
assert_equal [], Result.get_records
end
end
end
This test will fail if there are any results, but we have SOMETHING now.
5. Let's make the test succeed:
change the inside of the test:
def test_get_records
results = Result.find(:all)
records =Result.get_records
assert_equal results.size, records.size
records.each do |record|
assert_equal Result::BuildStatus, record.class
end
end
6. It seems that get_build_statuses is a more appropriate name for get_records.
rename it.
7. Now let's go through the block content:
move it to separate method:
def get_build_status
...
end
and change Result.get_build_statuses to:
def self
find(:all).collect do |result|
result.get_build_status
end
end
make sure the tests are still working
add test for get_build_status (it should return a BuildStatus),
check it's contents
i.e.
def test_get_build_status
result =Result.new
result.build_url = 'whatevr yo use'
build_status = result.get_build_status
assert_equal '...', build_status.module_name
...
end
8. move fetch inside the class, adding default parameter build_url, so
we can call
result.fetch insteadof fetch(result.build_url)
add test for fetch, make sure the tests are still working
9. separate table extraction
make sure the tests are still working
add test for it
10. separate table parsing
make sure the tests are still working
add test for it
11. refactor more
make sure the tests are still working
12. when you're done, you can drop some tests if they are no more needed.
Finally one really nice link:
This is James Edward Gray doing RubyQuiz along with testing.
class Result < ActiveRecord::Base
# default value for uri_str to allow calling result.fetch
# instead of fetch(result.build_url)
def fetch(uri_str = build_url, limit=10)
fail 'http redirect too deep' if limit.zero?
puts "Scraping: #{uri_str}"
response = Net::HTTP.get_response(URI.parse(uri_str))
case response
when Net::HTTPSuccess
response
when NetHTTPRedirection
fetch(response['location'], limit-1)
else
response.error!
end
end
#pick out the table
# response.body = data
# you can do without heigth:
# return data[table_start_pos...table_stop_pos]
return data[table_start_pos,height]
end
def get_build_status
# - move fetch inside Result class
response = fetch
# you can put .body into fetch, and combine the
following two into one:
# table = get_table(fetch())
# or even in one line:
# return parse_table(get_table(fetch()))
scraped_data = response.body
table = get_table(scraped_data)
build_status = parse_table(table)
return build_status
end
def self.get_build_statuses
find(:all).collect do |result|
result.get_build_status
end
end
end
EXAMPLE_RESULT = Result::BuildStatus.new('','','','')
# fill in the blanks
def setup @r = Result.new
end
def test_fetch @r.build_url = 'whatever' # fill in your example
assert_raise(RuntimeError) { @r.fetch('', 0) }
# zero is 'too deep'
assert_equal @r.fetch(@r.build_url), @r.fetch
# check for refactoring
end
def test_get_table
assert_equal EXAMPLE_TABLE, @r.get_table(EXAMPLE_PAGE)
end
def test_parse_table
assert_equal EXAMPLE_RESULT, @r.parse_table(EXAMPLE_TABLE)
end
def test_get_result
assert_equal Result::BuildStatus.new('',...),
r.get_build_status
end
end
end
On 11/22/06, Chris Gallagher <cgallagher@gmail.com> wrote:
thanks for the reply.
Zentest sounds pretty cool but there doesnt seem to be anything telling
people how to use it at the moment. how does one get zentest to scan
your code or how ever it does what it does?
--
Man's unfailing capacity to believe what he prefers to be true rather
than what the evidence shows to be likely and possible has always
astounded me. We long for a caring Universe which will save us from
our childish mistakes, and in the face of mountains of evidence to the
contrary we will pin all our hopes on the slimmest of doubts. God has
not been proven not to exist, therefore he must exist.
This is how I'd do it. I don't know the details, so your actual code
will be different. This code was not run at all, so probably there are
many typos/errors.
You'd certainly choose another names than I've chosen.
First of all, if you want to test, it's good to split the code in
small chunks so that each does a little bit of work. Thus you can test
them separately. I tend to put the code inside some class as well...
you know, the OOP
While adding tests, you want to change the code as little as possible,
to not broke it. When you have the tests in place, you can safely
change stuff (=refactor).
Two ideas to think about here:
* First, refactoring the code before you have tests is never a super
good idea. Writing unit tests that you know you're going to need to
refactor/replace/throw away is also. BDD and RSpec may be a
better way to go (any BDD folks want to pitch in with some ideas
here?).
* Second, once you've got your suite of tests written (either unit
tests or BDD style specifications), you should probably run your
tests under rcov. C0 coverage (like rcov measures) won't ensure
that you have all the tests you need, but it will help ensure that
you're not leaving chunks of code untested.
(Nice walk through below ... we can sure use more documents like
this.)
···
On 11/21/06, Jan Svitok <jan.svitok@gmail.com> wrote:
The final code could be a bit different from what is written in the
mail, as the code was written before the text, and I was too lazy to
fix it
Now the steps to the code below:
1. minor corrections:
- at the end of the code 'end' is missing
- change |records| to |result| as you are iterating over Result(s)
- change each to collect as you seem to collect results (but I may be wrong)
2. move the whole each/collect block inside Result class as:
class Result < ...
def self.get_records
Result.find(:all).each do |result|
...
end
end
end
(you can omit the Result. before find)
We have a function that returns something for all Results
what remains at the bottom is
records = Result.get_records
3. The block doesn't return anything meaningful so:
- add just after class Result < ...:
BuildStatus = Struct.new(:module_name, :build_status_since,
:last_failure, :last_success, :build_number)
- add BuildStatus.new(module_name, build_status_since, last_failure,
last_success, build_number) at the end of the block (inside it)
4. Now we can add the first unit test at the very bottom:
comment out the records = ... line
and add:
if __FILE__ == $0
require 'test/unit'
class TestResult < Test::Unit::TestCase
def test_get_records
assert_equal , Result.get_records
end
end
end
This test will fail if there are any results, but we have SOMETHING now.
5. Let's make the test succeed:
change the inside of the test:
def test_get_records
results = Result.find(:all)
records =Result.get_records
assert_equal results.size, records.size
records.each do |record|
assert_equal Result::BuildStatus, record.class
end
end
6. It seems that get_build_statuses is a more appropriate name for get_records.
rename it.
7. Now let's go through the block content:
move it to separate method:
def get_build_status
...
end
and change Result.get_build_statuses to:
def self
find(:all).collect do |result|
result.get_build_status
end
end
make sure the tests are still working
add test for get_build_status (it should return a BuildStatus),
check it's contents
i.e.
def test_get_build_status
result =Result.new
result.build_url = 'whatevr yo use'
build_status = result.get_build_status
assert_equal '...', build_status.module_name
...
end
8. move fetch inside the class, adding default parameter build_url, so
we can call
result.fetch insteadof fetch(result.build_url)
add test for fetch, make sure the tests are still working
9. separate table extraction
make sure the tests are still working
add test for it
10. separate table parsing
make sure the tests are still working
add test for it
11. refactor more
make sure the tests are still working
12. when you're done, you can drop some tests if they are no more needed.
class Result < ActiveRecord::Base
# default value for uri_str to allow calling result.fetch
# instead of fetch(result.build_url)
def fetch(uri_str = build_url, limit=10)
fail 'http redirect too deep' if limit.zero?
puts "Scraping: #{uri_str}"
response = Net::HTTP.get_response(URI.parse(uri_str))
case response
when Net::HTTPSuccess
response
when NetHTTPRedirection
fetch(response['location'], limit-1)
else
response.error!
end
end
#pick out the table
# response.body = data
# you can do without heigth:
# return data[table_start_pos...table_stop_pos]
return data[table_start_pos,height]
end
def get_build_status
# - move fetch inside Result class
response = fetch
# you can put .body into fetch, and combine the
following two into one:
# table = get_table(fetch())
# or even in one line:
# return parse_table(get_table(fetch()))
scraped_data = response.body
table = get_table(scraped_data)
build_status = parse_table(table)
return build_status
end
def self.get_build_statuses
find(:all).collect do |result|
result.get_build_status
end
end
end
EXAMPLE_RESULT = Result::BuildStatus.new('','','','')
# fill in the blanks
def setup @r = Result.new
end
def test_fetch @r.build_url = 'whatever' # fill in your example
assert_raise(RuntimeError) { @r.fetch('', 0) }
# zero is 'too deep'
assert_equal @r.fetch(@r.build_url), @r.fetch
# check for refactoring
end
def test_get_table
assert_equal EXAMPLE_TABLE, @r.get_table(EXAMPLE_PAGE)
end
def test_parse_table
assert_equal EXAMPLE_RESULT, @r.parse_table(EXAMPLE_TABLE)
end
def test_get_result
assert_equal Result::BuildStatus.new('',...),
r.get_build_status
end
end
end
This is how I'd do it. I don't know the details, so your actual code
will be different. This code was not run at all, so probably there are
many typos/errors.
You'd certainly choose another names than I've chosen.
First of all, if you want to test, it's good to split the code in
small chunks so that each does a little bit of work. Thus you can test
them separately. I tend to put the code inside some class as well...
you know, the OOP
While adding tests, you want to change the code as little as possible,
to not broke it. When you have the tests in place, you can safely
change stuff (=refactor).
The final code could be a bit different from what is written in the
mail, as the code was written before the text, and I was too lazy to
fix it
Now the steps to the code below:
1. minor corrections:
- at the end of the code 'end' is missing
- change |records| to |result| as you are iterating over Result(s)
- change each to collect as you seem to collect results (but I may be
wrong)
2. move the whole each/collect block inside Result class as:
class Result < ...
def self.get_records
Result.find(:all).each do |result|
...
end
end
end
(you can omit the Result. before find)
We have a function that returns something for all Results
what remains at the bottom is
records = Result.get_records
3. The block doesn't return anything meaningful so:
- add just after class Result < ...:
BuildStatus = Struct.new(:module_name, :build_status_since,
:last_failure, :last_success, :build_number)
- add BuildStatus.new(module_name, build_status_since, last_failure,
last_success, build_number) at the end of the block (inside it)
4. Now we can add the first unit test at the very bottom:
comment out the records = ... line
and add:
if __FILE__ == $0
require 'test/unit'
class TestResult < Test::Unit::TestCase
def test_get_records
assert_equal , Result.get_records
end
end
end
This test will fail if there are any results, but we have SOMETHING now.
5. Let's make the test succeed:
change the inside of the test:
def test_get_records
results = Result.find(:all)
records =Result.get_records
assert_equal results.size, records.size
records.each do |record|
assert_equal Result::BuildStatus, record.class
end
...........
..........
...........
.........
thanks for the long post Jan, youve given me a good insight into the way
that my code should be tested. I think that its hard to see how it works
exactly when you look at tests being done on other peoples code but when
its your own, it makes a bit more sense.
I was dreading the idea that Id have refactor it. Not something i've
done before as im very new to ruby code at the moment. In my original
post i actually omitted a lot of the code which might have been vital to
the way that you refactored it and now im a bit lost.
Heres the full code (only another few lines extra):
include REXML
def fetch(uri_str, limit=10)
fail 'http redirect too deep' if limit.zero?
puts "Scraping: #{uri_str}"
response = Net::HTTP.get_response(URI.parse(uri_str))
case response
when Net::HTTPSuccess
response
when NetHTTPRedirection
fetch(response['location'], limit-1)
else
response.error!
end
end
> thanks for the reply.
>
> Zentest sounds pretty cool but there doesnt seem to be anything telling
> people how to use it at the moment. how does one get zentest to scan
> your code or how ever it does what it does?
One of these days, maybe I'll write a newer version covering the
cool stuff like autotest and red-green that's been added lately.
The slides from my RubyConf*MI talk touch on Test::Unit and ZenTest,
but they don't really tell the whole story. I keep hearing rumblings that
video from the conference is going to be available, and then you can
hear the incoherent mutterings that accompany the slides.
···
On 11/21/06, Leslie Viljoen <leslieviljoen@gmail.com> wrote:
On 11/22/06, Chris Gallagher <cgallagher@gmail.com> wrote:
Les
--
Man's unfailing capacity to believe what he prefers to be true rather
than what the evidence shows to be likely and possible has always
astounded me. We long for a caring Universe which will save us from
our childish mistakes, and in the face of mountains of evidence to the
contrary we will pin all our hopes on the slimmest of doubts. God has
not been proven not to exist, therefore he must exist.
Never mind. Here's what you can do: I see you iterate over Results
twice. I suppose it's enough to reuse the the original Result (result)
in the outer cycle.
so instead of get_build_status you can name it update_build_status,
and replace ret. with self., remove the last line, and remove the
BuildStatus struct altogether.
Then you can create method update_build_status! that calls
update_build_status and save.
or you can stuff the save into update_build_status or in the class
method (self.get_build_statuses, that might be renamed to
update_build_statuses, and the each stays indeed as it is)
Whatever you're trying to achieve, state that in your tests. I mean,
if you want to have a particular Result have a particular values,
check them.
Another problem with tests is how you can live with external
dependencies - this time a) objects in DB b) actual pages that get
scraped. I tend to separate that stuff, so I can provide my fake
values - you can see it in how I did get_table - it doesn't call fetch
at all. the same with parse_table. At the end, you wire them together
with all the dependencies.
Somehow just massaging the code to be better testable seems to improve
its quality, in terms of readability, density, etc. Several times when
I changed code to be able to write the tests for it, I was just
watching how the code fell away
···
On 11/22/06, Chris Gallagher <cgallagher@gmail.com> wrote:
thanks for the long post Jan, youve given me a good insight into the way
that my code should be tested. I think that its hard to see how it works
exactly when you look at tests being done on other peoples code but when
its your own, it makes a bit more sense.
I was dreading the idea that Id have refactor it. Not something i've
done before as im very new to ruby code at the moment. In my original
post i actually omitted a lot of the code which might have been vital to
the way that you refactored it and now im a bit lost.
Heres the full code (only another few lines extra):
include REXML
def fetch(uri_str, limit=10)
fail 'http redirect too deep' if limit.zero?
puts "Scraping: #{uri_str}"
response = Net::HTTP.get_response(URI.parse(uri_str))
case response
when Net::HTTPSuccess
response
when NetHTTPRedirection
fetch(response['location'], limit-1)
else
response.error!
end
end
