The bugs that pass every check

Three bugs, all from the same production codebase, all found in one week:

- A form whose `__init__` assigns to `self.fields['fonction'].choices`
  crashed with `AttributeError` on every GET because `Contact.fonction` is a
  plain `CharField` — 500 in production.
- A `post_save` signal on `Mandat` created a `ConfigurationTVA` without
  passing the NOT-NULL `regime` FK — `IntegrityError` on every mandat
  creation.
- An `OperationTVAForm.clean()` that assigned `cleaned_data["montant_ttc"]`
  while `montant_ttc` wasn't in `Meta.fields` — Django silently dropped the
  value, NOT NULL violation at save.

Every one of these passed `./manage.py check`, passed its unit test suite,
and still reached production.

That last sentence is the reason django-test-doctor exists. It was built against AltiusOne — Django 6, DRF, PostGIS, pgvector, roughly 175 models, 150 forms, 202 serializers and 300+ URLs — and it now ships fifteen checks under one command.

The gap it fills is stated in one line:

./manage.py check validates runtime configuration. djlint validates template syntax. pytest validates whatever you remembered to test. Nothing validates the whole project at once.

What these three bugs have in common

They are not logic errors. Every one of them is a pathway bug: the code is correct in isolation and breaks on a route an actual user takes first.

A unit test for ContactForm tests is_valid() with data. It does not instantiate the form blank, which is what a GET does — so __init__ assigning .choices to a CharField never runs in the suite and runs on every page load in production.

A unit test for Mandat calls Mandat.objects.create(...) with the fields the test author cared about. The post_save signal fires, and if the test database happens to have a default ConfigurationTVA regime lying around from a fixture, it passes. In production it does not.

A unit test for OperationTVAForm.clean() asserts on cleaned_data. It is correct: the key is in cleaned_data. Django drops it at save(), which the test never calls.

The pattern generalises: the tests you write exercise the objects; the bugs live in the seams between them.

Instantiate everything, blankly

The cheapest check in the tool is the one that would have caught the first bug:

Layer	What it does
forms	Instantiates every Form / ModelForm blank + with empty data

That is it. Walk every form class in the project, call it with no arguments and with data={}, and report anything that raises.

It sounds too simple to be worth shipping, and it catches a genuinely common class of failure, because __init__ on a Django form is where people put runtime customisation — narrowing a queryset, setting choices, hiding a field based on a user — and every one of those lines runs before any validation and is skipped by any test that constructs the form with data it has already shaped.

The urls layer is the same idea applied to routes: crawl urlpatterns, probe every URL as anonymous / authenticated / staff / superuser. Four roles, because a permission bug is a bug at one role and correct at another.

Reading the source, because the runtime cannot see it

The third bug — cleaned_data["montant_ttc"] = … on a field absent from Meta.fields — cannot be found at runtime. There is no error. Django’s construct_instance() iterates the form’s fields and ignores everything else in cleaned_data, which is documented behaviour and usually what you want.

So forms_meta parses the source:

"""
This check parses each ``ModelForm``'s ``clean*()`` methods with ``ast`` and
reports every ``cleaned_data[<key>]`` assignment where ``<key>`` is a
NOT-NULL model column, absent from ``Meta.fields`` and not declared on the
form — **and** the form has no ``save()`` override to persist it manually.
"""

Count the conditions: four, and every one of them is there to suppress a false positive.

Assigning to cleaned_data for a key that is not a model column at all is normal — that is how you pass computed values between clean() and a view. Assigning to a nullable column is harmless. Assigning to a column that is in Meta.fields is exactly right. And a form with a save() override may well be persisting the value by hand, which is a legitimate pattern.

Only the intersection of all four is a bug. A check that fired on cleaned_data[x] = y alone would produce hundreds of findings on any real codebase and be switched off within a day.

The implementation is careful about what it cannot see:

try:
    source = inspect.getsource(form_cls)
except (OSError, TypeError):
    return  # no source available (e.g. generated via modelform_factory)

Forms built by modelform_factory have no retrievable source, so they are skipped rather than guessed at. Silence over speculation.

The check that made me want to write this

post_smoke POSTs to every CreateView and UpdateView with an auto-generated fixture. That closes the gap the urls check leaves — GET-only probing never reaches form.save(), which is where the second bug lived.

"""POST smoke: auto-generate a fixture, POST it, fail on 5xx.

Closes the gap the ``urls`` check leaves open — it only hits GET, so a
``CreateView`` that 500s only on ``form.save()`` (IntegrityError in a
``post_save`` signal, bad ``__init__``, missing NOT NULL FK, …) flies under
the radar.

Safe by construction: every request runs inside a ``transaction.atomic()`` +
``savepoint_rollback`` so the production database is **not** mutated even
if the view reaches ``.save()``. Run against a dev database anyway.
"""

Safe by construction… run against a dev database anyway. Both halves are correct and the second one is the honest one. The savepoint rollback undoes database writes; it does not undo an email sent by a post_save signal, a Celery task queued, or a webhook fired. The docstring does not oversell.

But the finding that justifies the whole layer is not the 5xx. It is this:

def _assert_created(self, url_name, route, view_cls, model_cls, pre_count, payload):
    """A CreateView POST returned success — did the row actually land?"""
    post_count = model_cls._default_manager.count()
    if post_count == pre_count:
        yield Finding(
            severity=Severity.CRITICAL,
            rule="post_smoke.stale_db.create",
            message=(
                f"POST returned 2xx/3xx but no new {model_cls.__name__} "
                "row was persisted. The view reports success while "
                "silently dropping the user's submission."
            ),
            fix_hint=(
                "Typical causes: a wrapping transaction rolled back "
                "after a signal exception, form_valid() forgot to call "
                "super() / form.save(), or an outer middleware swallowed "
                "a post-save exception."
            ),
            ...
        )

The view reports success while silently dropping the user’s submission.

That is the worst failure mode a web application has. A 500 is visible: the user retries, support hears about it, monitoring alerts. A 302 to a success page with nothing written is invisible on every axis — the logs say 302, the metrics say fine, and the person who filled in the form goes away believing they did.

Nothing else in the Django ecosystem checks for it, because checking requires doing the write and then counting. And the fix_hint is the part that turns a finding into a fix: three named causes, all of them things that actually happen.

The update variant is harder and the code shows the care:

# We only assert on fields we can confidently link back to the POST
# payload: ModelForm fields that were in the fixture. Everything
# else (computed totals, timestamps, etc.) is out of scope.
form_fields = _form_field_names(form_cls)
drift: list[str] = []
for name in payload.keys() & form_fields:
    pre_val = getattr(pre_instance, name, None)
    post_val = getattr(post_instance, name, None)
    expected = payload[name]
    # If the submitted value was identical to the current DB value,
    # an equal post-state tells us nothing — skip.
    if _values_equal(pre_val, expected):
        continue
    if not _values_equal(post_val, expected):
        drift.append(name)

Two exclusions, both necessary. Computed fields are out of scope, because a total_ttc recalculated by save() will never equal what the fixture posted and would fire on every form in the project. And a field whose generated value happened to match what was already stored carries no information — the post-state looks right whether the write happened or not.

That second one is the kind of reasoning that separates a checker people keep from one they silence.

Choosing your false positives, twice, in opposite directions

The tool makes the precision/recall trade explicitly, and — this is the part worth stealing — it makes it differently in two places.

In post_smoke:

# This misses views that return 200 on successful save
# (a minority pattern, e.g. JSON APIs) — those can be
# covered by per-endpoint tests instead. Trading a few
# misses for a 10× false-positive reduction on a normal
# CBV-heavy codebase.

Accept misses, kill false positives. The reasoning is that this check runs on every route in the project and produces CRITICAL findings; a critical severity that is wrong ten percent of the time trains everybody to skim past it.

In --diff:

"""
Findings with no file-path in ``location`` (e.g. "users:login") fall
through unchanged — we can't tell if they're relevant, so we keep them
visible. Better a false positive than a silent drop.
"""

Accept false positives, kill misses. Opposite call, and also right — because here the “false positive” is a finding that was already true, shown to someone who may not have caused it, while the “miss” would be a real regression invisible in the exact review where it could have been caught.

The general rule those two comments encode: the direction of the trade depends on what the reader does with a wrong answer. A wrong critical trains people to ignore criticals. A wrong reminder costs ten seconds. Deciding this per check, and writing down which way you went and why, is what makes a static analyser survive contact with a team.

Scoping to what you actually changed

"""``--diff <ref>`` — only surface findings that touch files changed since
a git reference.

The idea: on a large codebase a full ``doctor`` run can produce dozens of
findings. When a dev is reviewing their own PR they almost never care about
the existing debt — they want "what did *my* change introduce?".
"""

This is the feature that makes a fifteen-layer analyser usable on an existing project. Introduce any linter to a mature codebase and the first run produces two hundred findings, all of them pre-existing, and the team’s only rational response is to not run it again.

--diff main answers a different and much more actionable question.

The implementation is three git commands unioned:

commands = [
    ["git", "-C", root, "diff", "--name-only", f"{ref}...HEAD"],
    ["git", "-C", root, "diff", "--name-only"],
    ["git", "-C", root, "ls-files", "--others", "--exclude-standard"],
]

Committed changes, unstaged changes, and untracked new files. All three matter, because the most common moment to run this is before committing — the new file you just wrote is untracked, and a diff-scoped check that ignored it would miss exactly the code you are working on. --exclude-standard keeps .gitignored build output out.

And failure is graceful throughout:

except (FileNotFoundError, subprocess.SubprocessError):
    return set()
if out.returncode != 0:
    continue

No git, no repository, a bad ref — you get an empty set, which means no filtering, which means the full report. A scoping feature that hard-failed outside a git checkout would break every CI container that clones shallowly.

Configuration that acknowledges legacy

[tool.django-doctor]
enabled = ["*"]
disabled = []                          # or ["post_smoke"] to skip slow layers
fail_on = ["critical", "error"]        # what --ci should bail on
ignore = ["urls:admin:*", "forms:legacy_migrations.*"]

ignore with check:pattern globs is the pragmatic escape hatch, and it is scoped per check rather than per file — so you can silence the urls check on the admin without going blind to form errors in the same module.

fail_on separating severity from reporting is the other half: the tool can report a hundred warnings while failing the build on two errors. A CI gate that is all-or-nothing gets configured to nothing.

What it cannot do

Worth stating, because a tool like this invites over-trust.

The generated fixtures satisfy field types, not business rules. A form requiring a valid Swiss IBAN, a date after another date, or a foreign key to a row in a particular state will fail validation, and the check will see a 200 with form errors rather than a crash — a miss, not a false positive, but a miss.

forms_meta reads source, so it cannot see anything assembled at runtime. post_smoke needs a superuser and a writable database, which means it belongs in CI and not in a pre-commit hook. And the whole tool probes what is reachable from urlpatterns — a view wired up only through an include that a setting disables in the test environment is invisible.

None of that makes it less useful. It makes the boundary worth writing down, so that “doctor is green” is understood as fifteen specific classes of bug are absent, not as the project is correct.

What carries over

• Test the pathway, not the object. Blank-init, GET, POST, signal-save: the seams are where the production 500 lives, and unit tests walk around them.
• Assert that the write landed. A 2xx that persisted nothing is worse than a 500, because every signal you have says it worked.
• Four conditions, not one. A check that fires on the pattern rather than on the bug gets silenced in a day; the suppression clauses are the feature.
• Pick your false-positive direction per check, and write down why. A wrong critical costs you the whole tool; a wrong reminder costs ten seconds.
• Scope to the diff so the tool is adoptable. Nobody fixes two hundred pre-existing findings, and the untracked file is the one you are working on.
• Ship the fix hint with the finding. “Typical causes: …” is the difference between a report and a repair.