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 fixing the NodeBase tests.

Day 25: More Nodebase Improvements

Because I pulled the database code out of Everything::NodeBase in day 24, today I can pull the workspace code out of it too. Hooray!

There is workspace-aware code in Everything::HTML, node, nodetype, workspace, nodegroup, Everything::Nodeball, Everything::NodeBase, and Everything::Node. I want to start by subclassing NodeBase.

  $ cp lib/Everything/NodeBase.pm lib/Everything/NodeBase/Workspace.pm

Then there are a couple of modifications to the start of Workspace to inherit.

The way to go seems to be to edit NodeBase first, finding all of the workspace-aware methods. The first is joinWorkspace(). Hm, that’s a puzzler. There must be a database so that Everything::HTML can fetch the user to see if the user is inside a workspace, so this code can’t change too much.

The quickest option right now is to rebless the current object into the Workspace class. Yick. That’s not a general-purpose solution, but perhaps I’ll think of a better way to fix it later. After the bless, it makes sense to call joinWorkspace() again, so that Workspace can perform the necessary initialization. That’s a little scary, but it’ll let my subconscious mind work on finding better solutions.

getNodeWorkspace() can go away.

The next method that uses workspaces is getNode(). Workspace has to override this, but not entirely.

That’s it. Hmm. That seems easy. No tests fail in t/NodeBase.t (probably because they’re not very good). Changing the test file to test Workspace reveals one failure, where getNode() should return node zero when it receives a node_id of 0. After looking at the test, I think it’s because of the mocking strategy.

It’s time to fix the test file. This is yet another port to Test::Class.

The first few methods (the proxied ones) are easy to test. Unfortunately, Test::Class can’t do much with anonymous methods, so I have to use the string-eval form to create these test methods. That’s fine.

It’s more difficult to change these tests into good ones… but that’s okay too. I can write newer ones. (I think this after looking at the tests for new() and realizing that they work too implicitly.) Fortunately, the better separation of concerns makes this much easier.

I only made it through test_get_type() before the end of my day. That’s okay. I think all of the tests in the system still pass. Yes, all 7615 of them.

I will check in my changes and the new test file, in progress, but I won’t switch over to the new test style yet. That’s fine. All of the tests still pass but I get to check in the new code as revision #881.