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
man sqtl suggests:
$ sqlt -f MySQL -t SQLite all.sql > sqlite_all.sql
That command takes a while. (all.sql is a
catenation of all of the .sql files in
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
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' );
Can't locate object method "new" via package "Everything::DB::sqlite".
It needs an extra line:
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
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
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
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
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.
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.