Clean Test Class Code Review

As a bonus Xmas gift this week, I completed my part of a code review of a developer’s code before heading out Friday. The code review primarily covered a JSF backing bean/controller and a corresponding test case. Even though the tests had mostly been written after the test case was just a joy to read:

  • Clear test names.
  • Only 2-4 lines of code for most of the tests.
  • Thorough testing of negative conditions like <div class="codecolorer-container text vibrant overflow-off" style="overflow:auto;white-space:nowrap;">



    • 99% unit test coverage according to Clover.</ul>

    The only thing I was able to catch was a


    method that had been used when writing the tests to output something to the console. The method was still hanging around and didn’t actually do anything because the


    had already been removed.

Kicking Off With A Press Release At Amazon

Listening to an interview on Software Engineering Radio with Amazon’s CTO, Werner Vogels, a novel idea to chartering a project was explained. At Amazon instead of the traditional project charter or having a team just start with playing around with new technology, they require as a first step that the team writes a press release for the new feature. That way the new features on Amazon always start out focusing on how to appeal to the user. I’ve been exposed to this before with Luke Hohmann’s Product Box game. Might be great idea to try one of our next projects.

Fun With Nerf

Sometimes you just have to have a little fun. Nerf arms race, anyone?

A few days ago my developers explained that they were being randomly shot with nerf darts in their collocated cubicles by the QA tester. Next day, one of the mainframe developers picks up a nerf gun. The web developers were a little grumpy about being shot at. Manger must fix problem.

That involved stopping by Target on a lunch break and picking up three nice nerf guns including a Bazooka which shoots 60 feet in the air. I then snuck the munitions to the web developers and left them to their own devices. One of the three declared that he was only going to set his up in his cube as a deterrent. For the other two I think they had some fun sending a few darts and bazooka rounds over the cubes.

Even in corporate IT development shops it helps to relive a little dotcom fun. I still miss the foosball tables myself.

Tomorrow at the standup for this project I’ll have a simple explanation:

**“Yesterday, I fueled an arms race and provided nerf munitions to three web developers.”

“Today, I’ll be participating in a code review.”

“My impediment is I don’t feel comfortable submitting a nerf expense report to Finance.”

Build Working With Hudson

We’ve had an issue with one of our project builds that cruisecontrol can’t successfully checkout from CVS using maven 2. We went ahead and just manually logged into the build box and just ran a

cvs update

before each build. It works, but it’s clunky.

I tried luntbuild about a month or so ago, but I couldn’t get it to build. Then yesterday I pulled down Hudson, after seeing a short link to it on Cote’s blog. Web based, helpful documentation, and a pretty nice looking interface. After a few hours of troubleshooting I got our problematic build running successfully. The only disadvantages that I haven’t resolved yet seem to be:

  • The emails it sends out don’t include a lot of information, you have to link back to the web site to see the details.
  • Merging in checkstyle reports doesn’t seem to be an option.
  • Linking in artifacts like clover reports isn’t done on a simple link from the front of the build page, you have to navigate through the directory structure.

Not sure if we’ll just move to Hudson for our default continuos integration server, but I think cruisecontrol is getting outpaced by numerous other options.

Update on Crucible

Peter Moore from Cenqua Software stopped by for a customer visit late last week. Turns out he’s in the Bay area for a few months and Sacramento is just a short jaunt down the road.

We’ve been beta testing Crucible, their code review tool, since June. We’re pretty happy with it as it fits the lightweight toolset quite nicely. It also comes bundled with Fisheye, which at first glance seemed like a simple browsesable CVS repository. Fisheye has turned out to a useful addition alerting people to the direction of our entire corporate codebase is going. Everything from noticing direct JDBC calls in some old code to people deciding to check a PHP project into:


Anyway Pete asked for our overall impression of Crucible, features that we thought might be missing, things that bugged us about the product. We have had a few things:

  • Currently Crucible sends an email after every comment, this ends up being a lot of email for coders used to only getting 5-10 emails per day.
  • When setting up a review it defaults to the latest diff of a file instead of the whole file. Since we’re not nearly that rigorous it makes more sense to review the entire file when we do a code review. You can switch it to the whole file, but you have to remember a few extra clicks.
  • The whole thing has a nice AJAX flavor, simply click anywhere in the code and add a comment right away, but some of the button labels and layouts still confuse a few of our developers. So a little more usability polishing might be in order.

None of these issues are really big deals for us and given the functionality available even in June we would have gladly purchased it at that point. So if you’re looking for a really lightweight web based code review tool, Crucible is worth checking out.

Sounds like it will come out of early beta in the near future so more people can take a look at it for themselves. Without necessarily meaning to they’ve carved out a small niche with a lightweight review process versus many of the heavier process tools. And it’s really nice as I’ve said before to deal with a small shop of pure developers who build tools for other developers.