>Just the Facts.

>What we think we know about our code, and what our code looks like to others and a system are usually two different things. In many cases we think we have a great design, are following good coding practices. However the reality may be far different than we think. This is where tools like PMD, CheckStyle, and FindBugs come into play. They can knock that chip right off our shoulder, and reveal some cold hard facts.

I’ve recently enabled PMD’s DRY task and FindBugs in the WTP Source Editing’s continuous integration builds. The main reason is to improve the code that we have, and to try and head off any potential bugs before they get reported. All of these CI builds are run using the Athena Common Builder, and when the analysis is complete the Jobs will have report links like the following available (the necessary ant scripts have been contributed back to the Athena Common Builder proect):

The screen shot contains the results of the Psychopath XPath 2.0 bundle analysis. Not everything here is actually a bug, and many of the duplicate code warnings are generated by the Java Cup Parser that is used. So each team will have to decide if it is actually a bug or not. I was pleasantly surprised to see once we eliminate the generated code, that there are only a handful of Duplicate code warnings.

One thing to be careful of when adding FindBugs to your builds, is that it can take a while to run. This highly depends on how many bugs it finds, and how large your code base is. For the JSDT build, I had to limit this to just the jsdt.core bundle, otherwise the project analysis timed out. Some may want to do these analysis reports as separate jobs or include them in their longer running Integration builds. Personally I like them as part of the CI job, and it gives an added incentive to fix these issues.

If you are part of the Continuous Integration game on Hudson, the DRY and FindBugs plugins do contribute to that game. Fixing warnings and bugs reported here can give you some bonus points.

So if this can be beneficial, why don’t we do it or make it a requirement?

The main reason is time. It does take a bit to get these setup in the build. Its one of the reasons I contributed the Ant scripts I created back to Athena. To make it easier for others to reuse them and add the reports to their builds. I also think sometimes we are so concerned about adding that new feature, or hitting a particular milestone we don’t take the time to make sure the code we write is the best that it can be. This does tend to cause us problems later on. Having these reports as part of the build, makes sure they are always visible to adopters, and the community in general. It can help give more insight into the actual stability. It also gives a very easy entry point for somebody to provide a patch for code. See a problem from a FindBugs report that has not be addressed, fix it and supply a patch.

The state of the code is far different than we think without these reports.

This entry was posted in build, eclipse, release engineering, testing. Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )


Connecting to %s