A broken test suite, a tree and finding a rabbit

I’m not always the best at leaving well enough alone. For years our test suite cleaned up after itself with all the elegance of a toddler tidying a bedroom by sweeping everything under the rug — it finally got to the point that it felt like it failed more than it passed (because it did - we had stats). Tests had always failed on occasion, but over the last 12 months it went from occasionally to most of the time, so one afternoon I thought I would have a look. I had 90 minutes left before I stopped.

Background: the backend here is TypeScript, talking to Postgres through Objection.js (a library that the original developer is no longer supporting), with Mocha running the tests. clearDatabase() was our test suite’s cleanup step — a helper that, when called, truncated every table, and then reseeded the database so the next test would start from a known state. It wasn’t automatic: something had to actually call it, usually in an after()/afterEach() hook. Our VP had made multiple attempts to remove this, reducing the number of times it was called in the test suite, but before this change we still had >200.

The problem we’d been living with

clearDatabase() was there when I joined the company. Coming from Ruby and RSpec, it felt insane that transactions weren’t standard, but this was the test suite and it was hard to change — the kind of code nobody wants to be the one who breaks. Previous attempts had pruned the easy wins, but the mountain still loomed. It “worked,” in the sense that the tests mostly ran and mostly went green, which is a very low bar we had somehow decided was good enough. However, as we added features, the failure rate kept climbing without anyone really noticing, because it had always failed a bit.

What did this mean? We had tests that failed in isolation but passed in the full run. When you start writing a new test you might have the database filled with the past 100 inserts, or it might be newly reset — there was no way to tell. I was used to test isolation; clearDatabase() did not give that, since it only ran when called. Further to that, it appeared to be the cause of our test failures, and we had no clear reason why. Sure, some failures were obvious (e.g. counts that didn’t match), but most were just random timeouts.

I never believed that the clearDatabase() call caused the timeout, but the timeouts happened when it was running. I took a sample of errors and asked Claude to investigate - it agreed, clearDatabase() was at the center of the problem. Knowing what I do know, I could likely have fixed the test failures and kept clearDatabase().

But a rabbit hole beckoned, and I still had 60 minutes left in the day, so down I went.

Reaching for a feature that was already there

I started looking at testing with Objection and how others solved this problem. I found pg-transactional-tests: Postgres transactions, rolled back instead of committed — the pattern I was used to, the pattern I already knew I liked.

It didn’t fit us. The library assumes a clean hook structure, describe blocks using beforeEach and it, but we had before blocks and shared state between tests. But the core concept could work — I saw the beginning of a solution: implement testing isolation by default with the library’s code, then layer that with conditionals to escape transactions when needed. It wouldn’t be pretty, but it was an improvement.

I started with transactionPerTest() — it wraps every it() globally via beforeEach/afterEach (same as the library). In the common case, you don’t call anything yourself — you just write a normal, independent test, and the rollback happens for free underneath you.

before() runs before beforeEach, so it’s outside the transaction

For this I added transactionPerDescribe() — this could be added to test files to create a transaction around a before block (which executed before any beforeEach blocks). Objection.js allows nested transactions, and that’s what this provided. It wasn’t pretty, but it moved me towards my goal: stopping rows being committed to the database.

// transactionPerDescribe()'s before() runs first, so the fixture
// is created inside a transaction and rolled back once the describe finishes.
describe("LpaCase#willsuiteStatus", () => {
  transactionPerDescribe();

  let lpaCase: LpaCase;

  before(async () => {
    lpaCase = await lpaCaseFactory({ status: "in_progress" });
  });

  it("returns the mapped status for in_progress", () => {
    expect(lpaCase.willsuiteStatus).to.equal("IN_PROGRESS");
  });
});

By default this doesn’t remove per-test isolation, either — the global per-test wrapping just adds a nested savepoint, which stops tests leaking into each other. But lots of our describe blocks actually depended on test leakage, and rewriting the test suite was not something I could do. So I added an opt-in flag, skipIndividualTransaction, to transactionPerDescribe, that stopped the per-test nested transaction. It still isolated the before block, but tests inside it could now share DB state, allowing the wider improvement without needing a major rewrite.

But leaking state was an issue, and having everyone remember to do this was a risk, so there’s now a custom ESLint rule (require-transaction-wrapper-for-before) that flags any describe-level before() without transactionPerDescribe() or skipTransactionWrapping() somewhere in its own or an ancestor describe. A before() that never touches the database can opt out with a plain disable comment — but by default, the lint rule assumes guilty until proven innocent.

Some tests need a real, aborted transaction

Fun fact, postgres aborts an entire transaction on any query error — not just the failing query. Every later query on that connection fails with “current transaction is aborted” until something rolls back, in full or to a savepoint. A try/catch around the failing query doesn’t save you from this on its own.

This was a hidden problem, made very visible once tests were executed inside a transaction: tests that deliberately trigger and recover from a DB-level error now just fail. We had seen this occasionally in the test run, but now it was a constant.

The fix, runInSavepoint(), runs the risky call inside its own nested transaction, and only rolls back that, leaving the ambient transaction healthy:

try {
  await runInSavepoint(() =>
    Country.query().insert({ id: duplicateId, name: "Second", code: "ZZ-D2" }),
  );
} catch (error) {
  // the savepoint was rolled back, not the ambient (per-test) transaction —
  // so this query still works instead of failing with
  // "current transaction is aborted"
  expect(await Country.query().findOne({ code: "ZZ-D1" })).to.exist;
}

It’s not just a testing trick, either — it’s the same pattern our production code already uses to catch a constraint violation mid-request without taking the whole request down.

Everything happens at the same instant

A smaller but more insidious issue is that records created in the same transaction all share the same createdAt timestamp. Why? Because Postgres’s now() is fixed for the whole transaction. This highlighted broken tests and code that passed due to default ordering logic that wasn’t explicit.

The rabbit at the bottom of the hole

Was the time invested worth it (2 hours dedicated to the task, plus 2 days of Claude fixing bugs in the background with occasional input)? It didn’t fix everything and there is still work to do, but we exposed assumptions our tests had been quietly making for years — about ordering, about hook timing, about what “isolated” actually meant — that a full table wipe had been papering over the whole time.

But yes, I think so, and the rabbit, well my boss was happy — he was the one who had been working on this previously. I can already tell that our CI system is passing more consistently (after a few post-merge fixes). Overall this will save me time, but more than that it touches what I most enjoy about programming, finding a bug and fixing it.

The full source for everything above — the transaction wrapper, the savepoint helper, the monotonic timestamp helper, and the ESLint rule — is up in the complete code reference, if you want to see it rather than take my word for it.