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 I ported several test files to the new format… and discovered a nasty bug in my SUPER module. Oops.
Day 20: How Quickly Can I Port Tests?
I managed to port two test files on day 19. Can I continue that pace today?
It depends on the size of the files. The first one is location,
which is slightly shorter than nodeball. I might get through it
all today.
After porting the basic Test::Class
headers and such, 212 tests run and 21 fail. There aren’t any skips. That’s
handy. (The headers represent 16% of the test file; that’s a nice percent.)
The first test method is for nuke(). There’s a
SUPER() call that needs testing. (location inherits
from node, so I may be able to inherit the test method too.)
The test takes a bit longer to port because it uses the really old mock
object style. One test fails there because of the SUPER() call.
With that fixed, 193 tests run and all of them pass. Now comes the real test;
inheriting this test’s parent.
Calling the parent test method first (and clearing the mock log on the mock database object before running the remaining test) makes 220 tests run. All of them pass. Success!
That work ported half of the test file to the new form. Next up is the test
for listNodes(). (This is a new method to location,
so there’s no inheritance.)
The code for this method is just a wrapper around
listNodesWhere(), so why does the test need to do anything
specific? I can test this quickly. Now 225 tests run and all of them pass.
listNodesWhere() takes a bit longer to port. I ended up reusing
some of the tests from listNodes(), but changing their
descriptions. There’s no sense in mishmashing tests for multiple methods.
I had a couple of failure on my first attempts due to re-using a single mock object for the node and the database. After decoupling them, 236 tests run and all pass. I think I’m done, in about 25 minutes.
How does the whole test suite look? 3584 tests run, with seven skips and everything else passing. Excellent. It’s time for another checkin. This is #858.
Next up is the nodelet node. After the minimal porting, 212
tests run and 6 fail. There were 12 skips. That’s not bad at all. This node
inherits from node, so inheriting test methods ought to work
nicely.
The first test to port is test_dbtables(). This failed on my
first attempt because I had forgotten to import Test::More. (I
keep thinking of a way to automate porting these tests — at least their
headers — but it always seems so much shorter to retype things. Maybe if I
were doing this all in one forty-hour week I would have less patience for
repetition.)
That’s a third of the node class, already tested with the new approach.
Next is insert(). With the test ported (and a SUPER()
call fixed in the node), 206 tests run and 4 fail. There are also some
skips.
Loading and inserting SUPER appropriately, as well as wrapping
test_insert__access(), test_insert_restrict_dupes(),
and test_insert_restrictions() fixes the failures and
warnings.
There’s no test for getNodeKeys(). Odd. There should be. After
adding one, 204 tests run and all of them pass.
I’m not sure if I can inherit the getNodeKeys() test, but it’s
worth trying. Yes — it worked. Now 208 tests run and all of them pass and I
can’t think of anything else to do here.
How does the whole project look? 3784 tests run, with the expected 7 skips. Here’s another checkin, #859.
I have time to port one more very small test file. opcode looks
small. How does it work? Oh, it extends htmlcode. Next.
document? It extends node. Great!
After porting the test header (and the test_dbtables(), 212
tests run and all of them pass. Excellent. That’s also all it takes to port
this code.
Everything ought to pass properly, but it’s time to run the whole test suite
again. Hm, now the mail, restricted_superdoc, and
superdoc tests fail — because I changed a SUPER()
call in document!
Those test files are all small, so I might as well port them too.
Everything works fine, except that mail has an extra dbtable.
That’s easy to fix. However, there’s an infinite loop in
test_dbtables() in restricted_superdoc. I think it’s
a bug in SUPER.
I don’t have time to fix it now, so I modified the class to call
SUPER::dbtables() instead. That fixed everything. Now 4616 tests
run, 7 skip, and everything else passes. Yes, that’s over a thousand new tests
today. Excellent. This is checkin #860 and the end of a day.
(I fixed SUPER on the weekend.)
>> This 30-day project explores the refactoring of a legacy system
To date all it has done is explore some [very high level] refactoring of a test suite. I was really hoping to see more detail, and more involved information regarding what the code base looked like, what changes were made, how the code was tightened/cleaned etc.
Granted the test suite needs to be up to par before major changes can take place, but 2/3 of this project is now complete (time-wise) and all we've seen is endless "Converted $n tests. $something failed, but I fixed it easily. $n tests now run". There's been really nothing interesting, or at least not worth 20 blog entries about. There's been no real meat.
I can understand your frustration. I didn't know at the start of this project that I'd spend two weeks porting test cases -- that's the risk of refactoring a big project instead of starting a new one.
Fortunately, the last week or so of the project looks like it will make much larger code changes -- not just test changes.