Does this test prove the bug is fixed?

DecisionExpert4 min read20 min hands-on#concurrency#sql

The idea in 30 seconds

A bug fix arrives with a test that fails before the fix and passes after it. That proves the test reproduces something, not that it reproduces the bug. Ask whether the test can produce the conditions the bug needs (concurrency, commit, the real database engine) and whether the fix lives where every code path must pass through it.

Helps to know first: Race conditions and correct state transitions (coming soon), Isolation levels and anomalies (coming soon), Constraints and data integrity, Benchmarking and performance measurement (coming soon)

The situation

Support reports customers with two accounts under the same email. Some were created a second apart, by a double-clicked signup button; some differ only in case, such as asha@example.com and Asha@Example.com. A fix arrives.

Review taskPrevent duplicate accounts

Pull request: “Fix duplicate accounts. Signup now checks for an existing account first. Added a regression test.”

def sign_up(db, email, name):
    if db.query_one("SELECT 1 FROM users WHERE email = %s", [email]):
        raise AlreadyRegistered(email)
    db.execute("INSERT INTO users (email, name) VALUES (%s, %s)", [email, name])

# tests/test_signup.py  (test database: SQLite in memory, as for all our unit tests)
def test_second_signup_with_same_email_is_rejected(db):
    sign_up(db, "asha@example.com", "Asha")
    with pytest.raises(AlreadyRegistered):
        sign_up(db, "asha@example.com", "Asha again")

The test fails on main and passes on this branch.

Would you approve it? Write the review you would leave.

Your call

Model review

Request changes. The test proves that a second, later signup is rejected. Neither reported bug is that.

1. The check-then-insert is the race that causes the double-click duplicates. Two requests can both run the SELECT, both find nothing, and both insert. We ran exactly this pattern with pgbench on PostgreSQL 18.3: 4,000 signups over 200 emails produced 0 duplicates from one client and 7–8 duplicates from 8 concurrent clients, in each of three runs. The new test runs the two signups one after the other, so it can never see the race. It would have passed against the bug.

2. Case is not handled. asha@example.com and Asha@Example.com are different strings, so the check finds nothing and the second account is created. The test only uses one spelling.

3. The test runs on a different database. SQLite does not enforce varchar lengths, stores text in integer columns, and has foreign keys off by default. Whatever this test proves, it proves about SQLite.

4. The rule lives in one code path. The admin import, the API, and any future signup path must each remember to call this check.

What I would ask for instead:

Clean up existing duplicates first. Find them with SELECT lower(email), count(*) FROM users GROUP BY 1 HAVING count(*) > 1; and decide, with support, how to merge each pair.
Enforce the rule in the database, without blocking signups while the index builds:
```
CREATE UNIQUE INDEX CONCURRENTLY users_email_lower_key ON users (lower(email));
```
If duplicates remain, this fails, and leaves an INVALID index behind that must be dropped before retrying. We hit exactly that with one leftover pair:
```
ERROR:  could not create unique index "users_email_lower_key"
DETAIL:  Key (lower(email))=(asha@example.com) is duplicated.
```

Insert with the constraint as the arbiter, and turn a conflict into the “already registered” result:

INSERT INTO users (email, name) VALUES ($1, $2)
ON CONFLICT (lower(email)) DO NOTHING
RETURNING id;   -- no row returned means already registered

Tests against PostgreSQL: one for different case, and one concurrency test that fires the same signup from several connections at once and asserts exactly one row exists afterwards.

Compare it with yours. Did your review catch the main problems?

How to review a regression test

Does it reproduce the bug’s conditions, or just its symptom? “Two accounts with one email” is the symptom. The conditions were two signups overlapping in time, and two spellings. A test that creates neither cannot fail for the original reason.

Would it have failed before the bug was introduced, for the right reason? “Fails on main” is necessary, not sufficient. Here it failed on main because there was no check at all, not because of the race.

Does it run where the bug happens? The same engine and version, the same isolation level, real commits when commits matter.

Is the fix where every path goes through it? Rules in application code protect only the code that calls them. A constraint protects every path, including ones not written yet.

Check yourself

The new test fails on main and passes on the branch. Why is that not enough?
Show answer
It shows the test detects a missing check for sequential signups, not the reported bug. The duplicates came from concurrent signups and from case differences, which the test never produces.
I could explain this without looking
Why does CREATE UNIQUE INDEX CONCURRENTLY need the duplicates cleaned up first, and what happens if you skip that?
Show answer
A unique index cannot be built over rows that already violate it. The build fails, and because it was concurrent it leaves an INVALID index that still slows writes until it is dropped.
I could explain this without looking
With the unique index on lower(email), what happens when two signups for the same email arrive at the same instant?
Show answer
Both try to insert; one succeeds, and the other conflicts on the unique index. With ON CONFLICT ... DO NOTHING it inserts nothing and returns no row, which the application reports as “already registered”.
I could explain this without looking

Sources

Where each claim on this page comes from.

PostgreSQL documentation: CREATE INDEX

Primary· Building Indexes Concurrently

A failed CREATE UNIQUE INDEX CONCURRENTLY leaves an invalid index behind, which still adds overhead to writes and should be dropped before retrying.

PostgreSQL documentation: 11.7 Indexes on Expressions

Primary· §11.7 Indexes on Expressions

A unique index on lower(column) prevents values that differ only in case.

PostgreSQL documentation: INSERT

Primary· ON CONFLICT Clause

ON CONFLICT can name the unique index or expression to arbitrate on, and DO NOTHING skips the conflicting row.

SQLite documentation: Datatypes In SQLite

Primary· §3 Type Affinity

SQLite's typing rules differ from PostgreSQL's.

The situation

How to review a regression test

Check yourself

Sources

Go further