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 get just one single test running against the live, not a mocked, database.

Day 29: From The Database

It’s almost the end of the project, and I want to be able to run tests out of the database. To do this, I need to convert the existing MySQL schema into a SQLite-capable schema and see if I can install the nodes to it.

Why SQLite? It’s really easy to test against; just create a database file, make a new copy for each test, and go. It’s one dependency and it doesn’t rely on setting permissions or creating a test account though it does use the standard SQL/DBI interface.

It would also be nice to support SQLite as a single-user backend.

Another module worth knowing is SQL::Translator. I used this a couple of years ago when I put together the rudimentary (and probably fairly unused) PostgreSQL interface to Everything. It’s not perfect, but it worked well enough that I’m happy to try it again. (DBIx::DBSchema looks like a good second bet too.)

The module comes with a file called sqlt that can perform schema translations. man sqtl suggests:

  $ sqlt -f MySQL -t SQLite all.sql > sqlite_all.sql

That command takes a while. (all.sql is a concatenation of all of the .sql files in tables/.)

The created file looks good. Hopefully it will work with sqlite3, the command-line utility:

  $ cat tables/sqlite_all.sql | sqlite3 ebase.db

Hey, that worked. There were no egregious errors in the syntax. Excellent. Can I insert the base nodes?

  $ cat tables/basenodes.in | sqlite3 ebase.db
  # Create the nodes that we need to start the system
  INSERT INTO node VALUES (1,1,'nodetype',-1,'0000-00-00 00:00:00',
      '0000-00-00 00:00:00',0,0,0,0,'0000-00-00 00:00:00','rwxd','-----',
          '-----','-----',0,0,0,0,0);
  SQL error: unrecognized token: "#"

Hm, that’s a no. Does SQLite have a comment character? I can remove those lines temporarily.

  $ cat tables/basenodes.in | sqlite3 ebase.db
  INSERT INTO node VALUES (...)
  SQL error: PRIMARY KEY must be unique

Hm, that’s not good. I bet removing the primary key and letting SQLite assign it directly will help.

  INSERT INTO node VALUES (NULL,1,'nodetype', ... );

Yep. How does it look to SELECT from the database now?

  sqlite> select * from node;
  1|1|nodetype|-1|...
  2|1|node|-1|...
  3|1|setting|-1|...
  4|1|nodetype|-1|...
  5|1|node|-1|...
  6|1|setting|-1|...

Now that’s odd. Were the comment errors just warnings?

  $ rm ebase.db
  $ cat tables/sqlite_all.sql | sqlite3 ebase.db
  $ svk revert tables/basenodes.in
  $ cat tables/basenodes.in | sqlite3 ebase.db
  ...
  SQL error: unrecognized token: "#"
  ...
  sqlite> select node_id, title from node;
  2|node
  3|setting

Hm, that’s not it either. Odd. I bet taking out the comments will fix everything though; I just have to make sure to clear the database first.

  $ rm ebase.db
  $ cat tables/sqlite_all.sql | sqlite3 ebase.db
  $ cat tables/basenodes.in | sqlite3 ebase.db
  sqlite> select node_id, title from node;
  1|nodetype
  2|node
  3|setting
  sqlite> select nodetype_id, sqltable from nodetype;
  1|nodetype
  2|
  3|setting

That’s much better. I can live with this. I checked in my changes to tables/basenodes.in as #889.

In theory, I should be able to use ebase.db now in the tests to create and query nodes. Obviously I also need a way to create it and populate it during the tests. There are a couple of options.

First, I could create a test library dedicated to managing this file. I could use it from the node test class, initializing a fresh library in the startup fixture. That could slow down the tests a bit, but it creates a fresh version of the database on every test without me having to worry about other dependencies.

I could also create it as part of the ./Build test command. That has the drawback of requiring me to run this command every time I make a change to the test database. I usually prefer to run my test files on their own — it’s a lot faster and it gives me much less extraneous information.

I could combine the two ideas and generate a test database during the build process, then copy that pristine database around to a new file before each test class run. That avoids some rework, but alleviates the problem of the test data getting stale or extra data from a failing test making other tests fail as well.

Before doing any of that, I want to make sure that the SQLite interface works. There’s no point in starting to revise tests to run against a live database unless all of the other pieces work; otherwise I’ll spend too much time debugging errors that could be in too many places.

The first step is to load it:

  #!perl

  use strict;
  use warnings;

  use Everything::NodeBase;

  my $nb = Everything::NodeBase->new( 'ebase.db', 1, 'sqlite' );

That fails:

  Can't locate object method "new" via package "Everything::DB::sqlite".

It needs an extra line:

  use Everything::DB::sqlite;

Now there’s another error:

  Can't locate object method "loadNodetypeModule" via package
      "Everything::DB::sqlite" at lib/Everything/DB.pm line 46.

This is because when I factored out Everything::DB, I missed that loadNodetypeModule() stayed in Everything::NodeBase. It’s generic enough that it should stay where it is, but that means that buildNodetypeModuless() needs to refer to the nodebase object somehow. Grr.

The simplest approach I can think of right now is to add it as an attribute of the database object. (That has circular reference problems for garbage collection… so it has to be a weak reference.) A better approach is to create a method to return a list of active nodetype modules, then let the nodebase build them itself. I like that better.

After a couple of minutes, I have that in place and compiling and the code fails elsewhere now. It’s time to run the tests again. I know there will be a couple of failures. It’s easier to clean those now than later, though, while the changes are small.

As expected, the nodebase and workspace tests fail. Removing the test for the proxied buildNodetypeModules() method and making fetch_all_nodetype_names() in the mock storage object return false fixes both sets of failures. Of course, that makes leaves those two methods untested, but that’s fixable.

There aren’t any tests for Everything::DB yet for some reason. That’s weird. At least I need tests for buildNodetypeModules in Everything::NodeBase to keep up the coverage there. They should be easy. In fact, there are only two of them. It takes a couple of minutes to write them (compounded by the fact that one of my cats ate too much catnip and really really wants attention), but they make it in as checkin #890.

I forgot to run the entire test suite (the other cat is napping peacefully on my bed, bless his destructive little heart), but it looks good so far. Everything passes, all 7671 tests.

With that in place, how is my little test program now?

  Can't locate object method "getNodetypeTables" via package
    "Everything::DB::sqlite" at lib/Everything/DB.pm line 573.

That’s because the method is in Everything::NodeBase. Hmm, will I have to split this too? No… it looks like it belongs in Everything::DB. (It relates to the storage of nodes within the system.) It moves.

The next error is:

  Can't locate object method "construct" via package "Everything::Node" at
  lib/Everything/Node.pm line 85.

That’s weird. What type of node is this trying to construct that it isn’t using the proper node class? Hmm, everything looks good through debugging, but it’s not blessing the node into the proper class. (Maybe I never enabled this.) That needs to happen, probably in assignType().

Now there’s another missing method:

  Can't locate object method "getRef" via package "Everything::DB::sqlite"
    at lib/Everything/DB.pm line 523.

Hmm. There’s no easy way to unentangle this. It’s probably a spot for the weakref. Too bad. That takes a couple of lines of code and now the test program runs.

How is the test suite? The nodebase and workspace tests fail again. Why? getNodetypeTables() moved. That’s fine. Those tests should move into the Everything::DB test class (if it existed). Hmm. Now it does. It doesn’t run yet, but there’s one test in it. That’s fine.

All the tests pass now (7657 of them). This is a good place to end the day. This is commit #891. I wish I’d made more progress, but I needed to fix this code. Perhaps tomorrow I can get one test working from the database. That’s a good goal.