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 finish porting all of the node tests and to clean up the mess!
Day 22: The Last Node Test Porting, Ever
Is it possible to port the remaining tests today? I hope so!
The first is for
nodeletgroup. It only needs to override
test_extends() (and can inherit it). With that in place, 334 tests
run and all of them pass. I’m sure the suite all passes too, but it’s always
good to check.
While they’re running, I noticed that I had forgotten to check in an updated
Build.PL with a dependency on a newer version of
commit #867. 6078 tests run now and all of them pass. The new test is commit
The next node class to test is
htmlsnippet. It extends
htmlcode, so it can probably just inherit the tests directly. I
made that happen and 210 tests run with no failures. Looking at the diff, this
actually removes code. The test suite grows, but there’s less code in
Everything all passes again (6284 tests), so this is checkin #869.
The next node class to port is
permission. This is a lot of
boilerplate, but I’m so close to the end that it’s really not worth
automating it. (I normally would, but here’s a free insight for the day: if
it takes five minutes to automate and five minutes to do and you’ll never
do it again, just do it.)
htmlcode. Again, 210 tests
run and all of them pass. I expect no less. I run the tests though and the
entire suite just to make sure. (While the entire suite runs, I prepare the
checkin.) Again, this checkin actually removes more code than it adds. Now
6490 tests run and pass at #870.
(Some of you might ask why I checkin these so frequently now when I checked in much larger chunks earlier. Checkins are basically free with Subversion, so the smaller the unit of work, the easier it is to see what changed and when. Also, the smaller the change, the less work it is to revert if I ever get into a spot where I can’t make the tests pass.)
There are two node classes left for which to port tests. First is
theme. It extends
nodeball, so there should be a
test_extends() override. (I wonder if I forgot those for the
other tests… I’ll check after this.)
theme tests get in an infinite loop in one of the
hateful XML methods in
nodeball. I wonder if importing
SUPER() will help. Explicitly using
Test::More in the
test code doesn’t. Explicitly using
SUPER doesn’t either. There’s
deep recursion in
applyXMLFix() in the
test_apply_xml_fix_no_fixby() method in the
This is the source of the
uninitialized value warning in
nodeball tests. Why? I wonder if fixing that would fix the
test. I fixed the warning with an
exists test in
applyXMLFix() (checkin #871).
The infinite loop is yet another
SUPER bug, I bet. Ugh.
Actually, it isn’t. It’s because of the evil, icky way that
applyXMLFix() as a
setting function. There’s no good way to fix that without
nodeball contain a
setting node, and
that’s a bigger refactoring than I can do now.
For now, I’ll just stub out
test_apply_xml_fix_no_fixby_node() in the
tests. I don’t like it, but it’s progress and it’s something I know I have to
This is a good place to use TODO tests, actually. Now 290 tests run and the four TODO tests fail, as expected, so everything passes. (Expected, temporary failures are okay. Marking them as TODO tells the test suite to keep track of them but not count them against you.)
How’s the full suite? 6776 tests now run, with no unexpected failures. (The harness reports everything as passing, which is nice.) This is checkin #872.
Finally, it’s time for the
They port quickly, as you might expect. 211 tests run and all pass (after I
remembered to use
Test::More in the new file). The entire suite
now has 6983 tests. Everything passes. Here’s checkin #873.
My last task today is to check that every node that needs it has a
Hm, I don’t see tests for
container. Weird. I’ll add that too,
while I’m at it. 212 tests run and all of them pass. (I had to port the
test_dbtables() code.) This is commit #874.
What’s missing inheritance tests?
test_dbtables() added, 212 tests run. Now the entire suite has
7400 tests. This is commit #875.
test_extends(). Now 213 tests
run and pass there. I missed
permission earlier. 211 tests run
and pass there.
restricted_superdoc gets the method. 213 tests
run and pass there.
superdoc is the same.
usergroup needs the method, and now runs and passes 322 tests.
workspace now runs and passes 211 tests.
symlink last time, for some reason. It runs and
passes 212 tests, after porting. Now 7615 tests run in the entire system. I
checked in the
symlink tests as #876 and the inheritance tests
That’s it for today. What should I do next time? It’s probably time to untangle the database code.