DEV Community

Tdude
Tdude

Posted on

Three holes in a green CI, and a lock for the free plan

What this is about. On a site we worked on recently, the test pipeline went green across the board, and we looked into what green actually covered. Three things turned out not to be covered at all. This is how we found and closed each one, on a free GitHub plan.

Why. A green CI tells you that the tests it ran passed. It doesn't tell you which tests it didn't run, or whether anything waits for it before you deploy.

The question

Once CI was green, we wrote the next step down as one line:

go on with test, CI and safety planning. Output a shortlist in order of priority

That gave us ten items, and the top two were about the pipeline itself, not about the code. So we started at the top.

Hole 1: tests that CI never ran

The backend is a handful of small services, and CI runs each one's tests in its own job. A couple of services had plenty of tests and no job at all. Nobody had removed them; they had just never been added to the list.

Adding them was one word each in the workflow file. Then the first run showed what they had been hiding:

  • A pile of tests errored instead of running. Their database tests looked for a Postgres address in a dedicated variable. One test file skipped politely when it wasn't set; the files that borrowed its fixtures didn't, and crashed on an empty URL.
  • The async tests failed because pytest-asyncio wasn't installed in CI. They were never broken; they were never run.

The fix was a small conftest.py with one rule: without a test database, skip every test that needs one. In CI, fail instead. The second half matters most. A broken database in CI should turn the run red, not quietly turn the database tests into skips.

def pytest_configure(config):
    global _db_ready
    url = os.environ.get(TEST_DB_ENV, "")
    _db_ready = bool(url) and _database_reachable(url)
    if not _db_ready and os.environ.get("CI") == "true":
        raise pytest.UsageError(f"CI: {TEST_DB_ENV} unreachable at {_redact(url)}")
Enter fullscreen mode Exit fullscreen mode

_redact drops the password from the URL before it reaches a log, because CI logs are read by more people than your secrets file is.

Hole 2: tests that built a different schema than production

In production, the service builds its tables by running its migrations when the container starts. Its tests built them straight from the models with create_all. Two roads to the same place, as long as nobody forgets a migration. Forget one, and the tests pass while production queries fail.

So we added one test: run the real migrations on an empty database, then ask Alembic to compare the result with the models.

diff = compare_metadata(context, Base.metadata)
assert diff == [], "models and migrations differ:\n" + "\n".join(map(str, diff))
Enter fullscreen mode Exit fullscreen mode

It found drift on its first run. Harmless drift, and the database itself needed no change. Still, a test that checks for drift and finds some on day one has earned its place.

Another service had the opposite problem. Its background jobs opened their own database connection from the real connection string, not the test one. In CI that pointed at an empty database, so the jobs failed, logged an error and moved on, and the tests passed. A few tests had in fact been passing because a background job failed silently. Now the jobs use the test database, or stay off unless a test asks for them.

Hole 3: nothing waited for green

The repo is private and on a free plan, and GitHub's branch protection, the thing that stops a red commit from merging, needs a paid plan there. So a commit could go red in CI and still be deployed a minute later.

Every deploy goes through one command, so the lock went there. Before anything is synced or deployed, a small script:

  1. checks that the commit being deployed is the one on the main branch,
  2. finds the CI run for that commit, and waits for it if it's still going,
  3. refuses unless it ended in success.

Docs-only commits don't trigger CI at all, so those get special handling: if every file changed since the last tested commit is documentation, that earlier run counts. Anything else without a run of its own is refused.

There's an escape hatch, ALLOW_UNTESTED=1, which prints a warning and goes ahead. It's for emergencies, not for when CI is slow.

We tested the script against a fake GitHub CLI in a throwaway repo, one case at a time: green, red, cancelled, still running, docs-only, untested code, not pushed, and the override. Then against the real thing, right after a push:

… waiting for Test Suite run on <commit>
✓ CI green on <commit>
Enter fullscreen mode Exit fullscreen mode

The same change fixed the order of pushes in the deploy command, too. It pushed the main repo before the submodule it points to, which is exactly the wrong way round.

What happened, how we fixed it, and how else it could be done

  • Services missing from CI: added to the job list, plus the one test dependency they needed.
  • Database tests that errored or skipped: a shared rule that skips without a database and fails in CI.
  • Schema built two ways: one test that runs the real migrations and compares them with the models.
  • Background jobs leaking out of the test database: pointed at the test database, or switched off by default.
  • Red commits deployable: a CI check inside the deploy command, with a loud override.

Other ways to make deploys wait for CI:

Option Better at Costs
A check in the deploy command (what we did) Free, works on any plan, covers the only way you deploy Only protects that one command; a manual deploy can skip it
Branch protection Enforced by GitHub for everyone, before merge Needs a paid plan for private repos on personal accounts
Deploy from CI (CI deploys only after tests pass) No way around it, and no laptop involved Server credentials live in CI, and you need to trust and maintain that path
A pre-push hook Catches it before it even leaves your machine Runs the tests locally on every push, and is easy to skip

Before you trust a green CI

  1. List your services and compare that with the list CI runs. They drift apart.
  2. Make "no database" a skip on your laptop and a failure in CI.
  3. If production migrates and tests use create_all, add one test that compares the two.
  4. Grep for anything that opens its own database connection; tests should own all of them.
  5. Make your deploy ask CI before it does anything, and make skipping that loud.

Green means a bit more there now than it did this morning. More from the workshop is on the Ship Log.

Top comments (0)