Just A Summary

Easy. Write tests that express your assumptions. It’s always good to make assumptions explicit. It’s better still to make them executable. Whenever you use something that’s a little bit surprising or hard to find in the docs, or when you do something that works around a bug, write a test.

Then, when the framework changes and tests are failing all over the place, test your assumptions. Hopefully there’ll be one or two failing tests there that point you to a solution to your problem. If there aren’t, then at least you know what’s not causing the problem. And, when you do find the underlying problem, you can add the new assumption to your tests for later.

I’ve just been working on Typo and found (rather usefully as it happens) that, if you add counter caching to a model, the model that holds the cache field will call all its save/update hooks whenever the cache field is updated.

Which is a little odd; it could be argued that simply adding what should be an ‘invisible’ cache field to a model shouldn’t go changing its behaviour in this way. So, I’ve created an assumptions_test.rb file and written the behaviour up as a test. Now, if David Heinemeier Hansson decides that this is undesirable behaviour and changes it, we’ll be able to see what’s going on.

The idea (as is so often the case) comes from Kent Beck. In this case, from Test Driven Development. One more book by Beck that deserves to be on every programmer’s bookshelf.

Why am I asking?

Recently, Typo, the ruby based blogging engine that this site runs on, underwent a strutural change that involved replacing 4 different tables in the database with a single table, and migrating all articles, comments, trackbacks and pages into this new table.

The migration was easy to write. Typo’s built on top of Rails, and Rails has tools for doing database migrations. It’s hard to test a migration though, so the first ‘big’ migration that moves articles into the new contents table just raises an exception saying “BACK STUFF UP before uncomment this line in the code and rerun the migration.”

Making that backup is a really good idea because, if you’ve turned on spam protection, the next migration accidentally throws away all comments on sufficiently old articles.

Oops.

When a comment is saved, it first ensures that the article it’s attached to isn’t too old (more than 14 days old by default). If it is, the comment doesn’t get saved. This is fine when you first attach a comment to an article, but less fine when you’re, say, adding a guid field to the article or, as in the case of the migration, moving the comment from one database table to another.

So, I’m adding a test or two to the test suite to ensure that we can modify comments on old articles. Once I’ve got that passing, we can be reasonably confident that, next time we do a migration that targets comments, we can be confident that all the comments will get modified appropriately, which can’t be bad.

What have I learned?

Because the change was a refactoring which adds no new functionality to Typo, I hadn’t written any new tests, just made sure that all the existing ones were passing. But no test suite is complete (unless you are testing a “Hello, world” program I suppose), so the issue that causes us (and anyone else who migrated to edge typo with spam protection turned on) pain wasn’t caught by the tests.

I don’t blame whoever wrote the tests in the first place for missing this particular case; there’ve been occasions when I’ve wanted to edit an old comment to correct a URL or something and have bounced off the spam protection, but it never occurred to me that this might be a problem for migrations until I saw the bug report.

What I have been forcibly reminded of is that no test suite is complete. So, when a bug comes in, you turn it into a test and add it to the test suite, then you should never introduce the same bug again.

Nothing new there of course. Surely everyone already knows this rule of testing. But there’s nothing quite like actually doing it again, and having a good deal of confidence that adding the test will save you from an entire class of problems in the future to help you internalize it.

I think that, before I do much more work on Typo, I’ll be taking a closer look at what is and isn’t tested in its test suite.

Typo Content Models

I’ve been attempting to follow the DRY Principle and rejigging Typo so that there’s fewer places with similar code to handle the cached html associated with a content object. Also, I’ve been thinking of how to build a more intelligent cache sweeper — at the moment, if you change a single article, every page in the cache is removed, which isn’t ideal. To that end I wrote a ContentModel mixin that adds a new class method:

class Article < ActiveRecord::Base
  include ContentModel

  content_fields :body, :extended
  ...

Declaring a content_field sets up a setter method that would notify any object observers that the content has changed and reset the associated _html attribute.

Which is all very well, but I think the time might have come to make a Content superclass. The various typo content models already have quite a few fields in common, and even Article, the ‘biggest’ of them doesn’t have too many unique fields, so using Single Table Inheritance a completely insane idea.

What does that buy us?

• Conceptually, it buys us a neater model, albeit at the expense of a slightly scruffier table structure (Rails can’t do multi-table inheritance yet).

• A unified ID scheme for our content models means that any putative table mapping content objects to currently cached files is going to be substantially simpler.

• Threading. Add parent_id column to the table and, because of the uniform ID structure, a comment gets to have a parent that is either an Article, another Comment or (possibly) a Trackback.

How could it hurt?

• The database table’s a bit scruffier.

• Writing the migrations could be fun.

• We’d have to make sure that the old articles/read/id URLs continued to work — Adding an old_id column is one way forward, but it strikes me that a better way to do it would be to extend the articles table until it had all the fields required then rename it appropriately and then copy all the other content objects into it.

Conclusions

Well, I think it’s worth doing. But it’s my idea, so I would say that wouldn’t I. If you’re a Typo person and you’re reading this, what do you think?