This 30-day project explores the refactoring of a legacy system. The Everything Engine is an aging software project that powers Perl Monks, Everything 2, and a few other websites. It suffers from poor design and maintainiability. Learn what it’s like to look over the shoulder of an experienced developer as he refactors, redesigns, and updates the code.

Today’s task is to test the new Workspace code.

The Workspace Tests

My last work yesterday was to create bare-bones Workspace tests. Today my job is to finish them. If I’m really quick, I might even be able to pull in some of the other workspace-related code from the rest of the system. (Having it all in one place is very nice.)

There are only three methods in Workspace, so this should be fairly quick. (The entire file is 202 lines long, with plenty of comments.)

The first method is joinWorkspace(). Due to the for-now-workaround I put in NodeBase, this is an uninheritable test method. That’s fine. It takes only seven tests to exercise this method completely. Now 135 run and all pass.

The second method is getNodeWorkspace(). It’s longer and more complex. I wanted to reuse the tests for it from t/NodeBase.t, but they never existed. Oh well.

It took me a few minutes to figure out what the code is doing. It loops through the nodes in the workspace, discarding nodes of the wrong type (if there is a type specified), and then applies a constraint checker to match against node attributes. There’s one small trick in the custom checker, and that’s to compare node ids appropriately.

There are a few different spots of difference to test:

  • with a type
  • without a type
  • returning no nodes
  • returning some nodes
  • checking multiple conditions for an attribute
  • checking single conditions for an attribute
  • checking no conditions

As it turns out, it only took five tests to make that work, though it took more time than I anticipated.

The final method to test is getNode(). Inheriting this works just fine. There are some changes though. My first steps in testing ran through the cases at the end, from calling the parent method to checking in the workspace for a workspaced version of the node.

The piece in the middle is a little more work. When a user selects a node through a SQL-like WHERE clause, the code finds the appropriate list of nodes, then grabs all the nodes in the workspace. It accepts any nodes that aren’t in the workspace, then adds all of the nodes that are in the results of searching the workspace.

If there aren’t any nodes, it returns. Otherwise, it orders them by the selected attribute, defaulting to node_id, and returning the first or last (depending on any sorting order).

I find it more useful to break down each method into a series of steps, then figure out the paths through those steps, testing my way through the function one step at a time.

To start, I need to pass in a WHERE clause. I made a new test method for this, test_get_node_with_where(). I’m still experimenting with test method organization, but keeping things together here seems nice.

It only takes four tests to get pretty good coverage. The sorting isn’t as exhaustive as I like, but this is clearly a test that needs a good set of data beneath it to work out nicely. I’m pretty satisfied so far that the tests work reasonably well.

How’s the suite? 7675 tests run and all pass. Nice. This is checkin #884.

(Looking through the rest of the code in the system, Everything::Node::node has the remaining workspace code, mostly related to loading and saving nodes. That code belongs elsewhere anyway.)