Thursday, August 16, 2007

Rails Unit Testing Without Fixtures

When testing models with complex relationships, fixtures can be a real pain. They require the details of a single test to be spread around several files; they aren't very dry, since you have to repeat a lot of details in each instance that may not be relevant to the situation you are trying to set up, and you are forced into putting a bunch of for unrelated tests into a single file.

The Test With Fixtures

Lets take the classic example of an Order item, that has_one product. If you are testing to ensure that the correct total is computed. With fixtures, it might look like:

def test_total_should_return_price_times_quantity
assert_equal 33.00, order_items(:first).total
end

To see that this is valid, you would have to look in order_items.yml to find out the quantity for item :first is 3, then look in products.yml to find that the price is 11.00.

There have been a number of alternatives to fixtures proposed, including Jay Fields' idea of using mocks to avoid database access, which seems like going a bit too far for my tastes; and fixture scenarios, which only solve part of the problem. Here's what I do:

The Test Without Fixtures

First, I create a build method for each of my model objects. This method will create a valid object with reasonable default values, except as modified by a hash parameter. For simple objects with no associations, it looks like this:

def build_product(attrs = {})
Product.new({
:name => "Test Product",
:price => 10.00,
}.merge(attrs))
end

For more complex objects, the builder should create any reasonable associated objects or use those past in:

def build_order_item(attrs = {})
OrderItem.new({
:quantity => 1,
:product => build_product,
}.merge(attrs))
end

These builder methods I usually put in test_helper.rb, since they ussualy need to be called from multiple test files. With these in place, out total test becomes:

def test_total_should_return_price_times_quantity
order_item = build_order_item(
:quantity => 3,
:product => build_product(:price => 11))
assert_equal 33.00, order_item.total
end

This test doesn't touch the database, and all the details relevant to what is begin tested are in one place.

Tuesday, April 17, 2007

Unproductive Haiku

No work done today.
The parts in which it's been cut
Are less than the whole.

Monday, April 16, 2007

My contract at a large government agency recently came to an end, and while I've been working on a project of my own which will hopefully be in beta soon, I've also been looking for more regular work. In the process, I've noticed an interesting dichotomy.

Most of the companies I've talked to have been unimpressive. Although their websites claim that they are passionate about quality, enhancing the customer experience, and hiring only the best and brightest, that's not the impression you get from talking to them. Their ads on one of the big job sites have a long list of technologies you are expected to have mastered. They do little to sell you on why you should be working for them and not one of the other body shops. The recruiter gives the impression they are just trying to find the right peg for the staffing hole in some large bureaucracy.

Two companies so far have stood out. Interestingly enough, I didn't hear about any of them from a job board. One, a group at Booz Allen, I heard about through a friend. Sure, they do typical government work, but the developers I talked to were all really pretty sharp. The other was at BrowserMedia, and although the location is not quite ideal (but doable), they sound like they do a lot of interesting work and really made me want to work there. I had heard about them from a post on the local ruby users group, but I notice they also have an ad on Joel on Software's job site, so I'm guessing their's going to be stiff competition.

I think I'm going to have to start paying more attention to the more focused job lists like Joel's.

Tuesday, April 03, 2007

Managing User Attention

One feature of the web application I helped developed for a client was a list of messages that was displayed when you first logged in. This feature had been in the system since day one, and included both system-wide messages entered by management, as well as messages auto-generated by the system. I wasn't originally responsible for the message feature, but when I first saw it, I estimated that, since most of the messages were not actionable, it would only be a matter of weeks before employees became habituated to the messages and ignored them. Sure enough, that is exactly what happened.

Although most of the messages were fairly irrelevant, roughly twice a year, management would put out an important messages that everyone was supposed to read. Once the managers realized that users were ignoring the message, they requested the feature be changed to make it more prominent. First, my coworker moved it to a separate page you had to click through before you got to the main page of the application. And of course, after a while, users became habituated to that as well. Then we started adding extra code to highlight the most important messages on the login page, which required the source that generated that page to be edited and redeployed each time the message was changed. Then we had to put them in bright colors. Almost a year ago when we got another request to change the login page message, and I decided to put an end to this attention arms race.

I modified the original message scheme to show messages in large, red letters or the first few weeks, then revert to the normal font. And I got rid of the other places we were displaying "special" messages. This actually went well beyond the user's original request, but in the change request form, I carefully documented what I was doing, and why. The system-wide message would be displayed prominently, and we wouldn't need to redeploy the application each time the message was changed.

Some time after putting that change in, I get a system change request asking us to modify the message. "We don't need a change request for this" I replied, "it isn't a software change - they can do it themselves now". I was told it had to go through the CR process anyway. After a bit of questioning, it comes out - apparently, management doesn't actually read the change requests that they routinely approve, and had no idea of what I had done. They too had become habituated.

Saturday, March 10, 2007

Cleaning Up Code

It was a single, 6000 line module, and it was a mess. The original developer had been gone for 6 months, and it had gone through several hands since then. It was constantly having problems, with new bugs discovered weekly, but nobody wanted to touch it because every time someone did, they would break something else. It landed on my desk with several as yet unfixed bug reports attached to it.

After looking through it I came to the conclusion that there was a relatively simple program buried inside, and went to work. There were no unit tests, but the main function of the program was to generate a number of output files. I could compare the correctness of my revised version by running both it and the original on the same data, and then comparing the output files.

I deleted redundant and unused code, refactored, and generally cleaned up the program. After untangling a particularly convoluted bit of logic, my comparison test failed. After half a day of research, I came to the conclusion that in this particular case, the original program had been in error. Two weeks later, I checked in a program that was 30% smaller, better documented, and much easier to follow.

The supervisor was a bit suspicious of this. His philosophy was that we should be changing just the bare minimum needed to implement a fix, since previous changes had a high probability of introducing new bugs. After the program passed the first round of verification using a single run (about 8000 records) with flying colors, he insisted on testing a whole two week cycle. Since I had been testing with several months worth of data, I wasn't particularly worried. When it passed that test, he finally authorized it to be moved into production.

Unfortunately, I never got a chance to finish cleaning up the code, since there were no additional bug reports in the six months that followed.

Friday, March 09, 2007

Feedback

I suppose we've all been in this situation. We have done something (revised our resume, changed hairstyles, whatever) and we ask a friend how it looks. "looks fine" we hear, or maybe they make some minor criticism - "you have a mispelling in the third sentance". The fact is, the people we deal with on a daily basis want to be nice to us, and are therefor reluctant to criticise or suggest improvements, even when that criticism is what we really need to do better.

Getting good feedback is hard, and yet it is extremely important to any effort to improve, whether improving your image or your software development process. And yet, so many organizations not only don't encourage feedback, they actively discourage it. This can be done in both subtle, and unsubtle ways.

One manager I know would dominate the conversation during the status meetings he held. He would prompt his underlings with specific questions about their work, then quickly move on. While this did help to keep the amount of wasted time down, it meant that issues that were not on the manager's agenda never got aired at all.

Now it is certainly true that developers with some problem to be addressed could have sent an email, or otherwise brought it to the managers attention, but "process" is one of those things the project manager is supposed to be in charge of this requires the developers to both recognize that there is a problem that can be solved or an improvement that can be made, and take the initiative to raise it. This is one of the reasons I think the sadly underused "retrospective" meeting can be so effective. It creates a forum specifically for addressing project wide and process issues that are often ignored in the heat of development.

Sometimes, even when issues are raised, they are repeatedly ignored by management, either by being put off "for later" or by not responding at all. Management in this case is sending a clear message that feedback is not wanted.

While not encouraging feedback can be a problem, even worse is discouraging it. At one organization I know of, a friend of mine once suggested some improvements to the current development process, and was told in no uncertain terms that his role was to accept the process as handed down by management, not to suggest improvements, and that if he continued with this "insubordination", he would be terminated. Needless to say, this organization was inefficient, had a lot of communications problems, and morale was poor to boot. Managers would complain that they had a hard time keeping track of the status of things. Hardly surprising if developers were fearful of giving any more information than what they had been specifically asked for.

Remember, everyone can have good ideas, and so its important to capture those ideas. Feedback is vital to any effort to improve. Encourage it.

Friday, December 15, 2006

rspec

After hearing Aslak Hellesoy talk about rspec on the rails podcast I figured I would give it a try. Its a neat way of seeing test driven development from a different perspective.

First, I tried creating a new spec for an existing model using the rspec_model generator, but this hung up (maybe because of some configuration issues on my XP box).

Then I got the following error when running a test:

c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/errors.rb:94:in `check': SQL logic error or missing database (SQLite3::SQLException)
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/resultset.rb:76:in `check'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/resultset.rb:68:in `commence'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/resultset.rb:61:in `initialize'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/statement.rb:158:in `execute'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/database.rb:211:in `execute'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/database.rb:186:in `prepare'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/database.rb:210:in `execute'
from c:/ruby/lib/ruby/gems/1.8/gems/sqlite3-ruby-1.1.0-mswin32/lib/sqlite3/database.rb:620:in `rollback'
... 11 levels...
from c:/ruby/lib/ruby/gems/1.8/gems/rspec-0.7.4/lib/spec/runner/context_runner.rb:22:in `run'
from c:/ruby/lib/ruby/gems/1.8/gems/rspec-0.7.4/lib/spec/runner/command_line.rb:26:in `run'
from c:/ruby/lib/ruby/gems/1.8/gems/rspec-0.7.4/bin/spec:4
from c:/ruby/bin/spec:18


The problem was apparently related to using embedded ruby expressions which used the model I had previously tried to create a spec for. The generator had overwritten my old model with a new blank model (I though generators were supposed to prompt before overwriting files), so the embedded ruby expression was not valid. Anyway, thinks seem to be working now.