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
The Workspace Tests
My last work yesterday was to create bare-bones
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
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.)