17 November 2009

Waddaya know, testing WORKS!

In my previous post (what? I’m doing another post just three days after my previous one? :o ), I mentioned that I was planning on adding QuickCheck support to graphviz. Last night, I finished implementing the Arbitrary instances for the various Attribute sub-types and did a brief test to see if it worked… and came across three bugs :s

Parsing my own dog-food

The property that I was testing was that parse . print == id; that is, graphviz should be able to parse back in its own generated code output and get the same result back. I decided to do a quick test on the Pos value type, as I figured this would be reasonably complex due to the usage of either points or splines. And yes, I was right that it was complex, as this revealed the following three bugs:

When printing the optional start and end points in a spline, they should be separated from each other and from the other points with spaces; I had used the pretty-printer <> combinator rather than <+> .

Lists of splines should have only semi-colons in between each spline and not a semicolon and a space: using hcat rather than hsep fixed this.

The parsing behaviour was initially to try parsing the Pos value as a point first and then a spline. However, if the spline didn’t contain an optional start or end point, then the parser would successfully parse the first point in the spline as a stand-alone point, and then choke on the space following it (or indeed, a spline consisting of a single point followed by another spline would also confuse the parser). Thus, testing for a spline-based position first fixed this.

Note that this is two printing-based problems and one parsing-based problem. The initial fix for the last bug, however, created another problem: as I alluded, a spline consisting of a single point is equivalent to a point, so all point-based positions would be parsed as a single spline consisting of a single point. The current parsing behaviour is now to only parse as a spline-based position, then convert it to a point-based position if necessary.

Rant about tests in packages

Duncan Coutts recently mentioned that QuickCheck is one of the packages that split HackageDB, due to the newer version 2 branch being incompatible with the (more popular) version 1 branch. My opinion is that this is a problem with how Haskell developers treat testing in their packages both from a user and from a distribution-packager point of view.

Let’s take hmatrix as an example. It uses both QuickCheck and HUnit for testing purposes. However, why does an end-user care about the tests, as long as the developer has run them? This introduces two compulsory dependencies to the package (which I have no problem with overall) that most people don’t need or care about. Some library developers include their tests (and the dependencies for those tests) in a separate executable that is disabled by default; however, due to how Cabal deals with this (Duncan has partially fixed the problem for Cabal-1.8), these “optional” dependencies are still required. I can think of several reasons why developers include the tests inside the main package in this way (listed in what I think is decreasing order of validity):

The tests use internal data structure implementations or functions that should not be publicly accessible.

The tests are also located inside the library as extra documentation about the properties of the library.

Convenience; everything is all bundled together, and if end users want to test the validity of the code it’s there for them.

Laziness: why should they bother separating it out when it makes it easier for them to do “cabal install” and run the test binary?

I myself have never run a test suite for a package that is not my own, and I wonder how many people actually do. I just find that this makes packaging libraries for Gentoo more difficult, and leads to the problems Duncan has been having on Hackage.

My approach

I have a darcs repository for graphviz, the location of which is listed in the Cabal file (and is displayed by Hackage). This darcs repository is publically available for anyone to get a copy of, so if people want to send me a patch with extra functionality they are able to get the latest stuff that I’m working on.

My testing files are located within this repository; the actual tests are defined an run in an external module from where the data structures and functions are defined. I can use it to test my code; if anyone else wants to test it they are able to grab it and do so. However, I am not going to include the testing module[s] within any releases of the library.

I believe that for many cases, this would be a much better approach on how to develop and distribute test suites. At the very least, if you are unable to extricate the tests from within your projects source files, using a pre-processor to remove them from the distributed tarballs might be a valid approach (I have no idea how hard/easy this would do though). This way, people who want to run the test suite can, and other people who trust the developers (like me for the most part) can do so without having to install dependencies that are for the most part useless.

Hmmm…. I’m sure there was a problem with a package which had something similar to yours (but only one flag) that kept pulling in quickcheck, so at the urging of users the developer removed the testing executable…

And I must say, that ChalkBoard looks rather cool, though your website lies in terms of what the latest version is… ;-)

In the end, I did something similar for graphviz (Spencer Janssen amongst others convinced me that hit should be part of the tarball). However, I also included a testing component into the overall library section, which is where the QC dependency gets added.

Welcome!

Greetings and salutations be upon thee to my little corner of what we like to charmingly call "the internet". This is where I put forth cunning arguments and repartee on such topics as Haskell, Gentoo, university... just don't ask me if I have a social life :s