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.

With nodegroup ported, I can port the nodeball tests. How well does inheriting from other test classes work? Not as well as I had hoped.

Day 19: Nodeball and Themesetting

With nodegroup ported successfully, the nodeball node tests should be easy to port. (That’s another reason why I spent three days working with nodegroup; I knew the next dependency.)

It’s time to move some files around again. With the new test file create (and the old test file running the Test::Class version), 333 tests run (with several skips) and 17 fail. That’s not bad at all.

The first test to add is for the dbtables() method; nodeball contains a settings node. (It should be a has-a relationship instead of a pseudo-is-a relationship, but that’s another deeper refactoring for later.) That makes one more test pass.

I also added a test_extends() method that inherits from the version in the node tests. While I was doing that, I noticed an error about a missing SUPER() function in the test class. I had made an error when installing SUPER::SUPER() in the nodeball class. Fixing that made several tests pass and revealed that actually 25 of my 334 tests failed. There are many fewer test exceptions caught now though, so these results are more sensible.

The first real test to port is for insert(). It seems pretty straightforward (and looks inheritable). After a minor port, 344 tests run and 26 fail. One of those is due to a SUPER() argument-passing change. The others are due to mocking changes. That’s fine.

How inheritable is this? The child has a getVars() call that the mock has to handle. Unfortunately, the parent test_insert() method also mocks SUPER(), so the code never gets out of the nodeball class. That means this method is uninheritable, with this approach. That’s fine. 330 tests run and 22 fail.

It takes a moment to port the tests for getVars(), and now 332 tests run and 22 fail.

The same thing happens for setVars() (except that I made a typo and had one test fail accidentally).

hasVars() is even easier to test. Now 334 tests run and only 21 fail. I’ve ported 64% of the test file already.

fieldToXML() has an awful implementation (with the not-quite-multiple-inheritance of setting). The test is simple though, and with it ported 324 tests run and 19 fail. Does the inheritance work? Before I answer that, I noticed that there aren’t any tests for the SUPER() call — specifically the necessary argument passing.

With a code change, that tests passes. Unfortunately, another test in this method fails… because I changed a calling style and forgot to update the test. After fixing that, 325 tests run and 18 fail. Nice.

Now for the inheritance question. After adding a SUPER() call to call the parent, 338 tests run and 19 fail. Why? Right — because the parent test method overrides SUPER() and the nodeball method can’t call the nodegroup parent method in that case.

There are two methods left to port. test_xml_tag() has four tests. It needs one more to check the arguments of the SUPER() call. Of course that fails, but editing the code fixes it. 309 tests run and 17 fail.

Is this inheritable? Nope. Again, the parent test method overrides SUPER(). It’s interesting to see how overzealous mocking gets in the way. If I’d pushed to test against a testing database, I wouldn’t have these problems now. (I’d have different problems. Oh well.)

Hopefully test_apply_xml_fix() will clear up most of the remaining problems. After porting it (and adding a SUPER() arguments test again), 295 tests run and 5 fail. Passing the arguments in the SUPER() call in the code cleans up four of the failures. The other failure is just a typo.

This method is again uninheritable due to the SUPER() mocking in the parent test method. That’s fine. I can live with 295 tests for now.

How is the rest of the code? 3144 tests run and they all pass. Great! This is checkin #856.

I have a few minutes remaining. How quickly can I port a really small test file? It takes about two minutes to port the themesetting tests, and now 231 tests run there where only 4 ran earlier. Now 3371 tests run and pass, and here’s another checkin — #857 — and the end of my day.

I like how quickly the smaller tests port and how much work I can reuse.