← Ascendy 한국어

backend

All tests green — and a validator that did nothing

· Ascendy Engineering


TL;DR

Source note. Distilled from a backend-team intake. Internal identifiers are generalized (UploadFinalizeRequest, capture_time), and Pydantic’s before/after behavior is a fact you can check in the official docs. Same adversarial review caught it vein as the post where one AI reviewed another AI’s PRs and caught four HIGH defects.

It was a small task

Store one client-sent timestamp into a timezone-naive DateTime column. Clients send an offset like Z or +09:00, but the DB and every other extraction path use naive values, so that offset had to be normalized to naive UTC. So I attached a validator to the Pydantic field:

@field_validator("capture_time", mode="before")
@classmethod
def _strip_tz(cls, v):
    if isinstance(v, datetime) and v.tzinfo is not None:
        return v.astimezone(timezone.utc).replace(tzinfo=None)
    return v

I wrote six unit tests. UTC Z, non-UTC offsets, null… all “verified,” all green. Looked mergeable.

Then one line came back from adversarial code review — “this validator doesn’t actually do anything in production, does it?”

Layer 1 — mode="before" wasn’t what I thought

before and after. By the names, it reads like an ordering question — before or after validation? It is about order — but that order is exactly what makes the real difference: the type of the value the validator receives.

A mode="before" validator runs before Pydantic coerces the type. So a capture_time arriving in an HTTP JSON body is still a string at that point — "2024-05-02T03:00:00+09:00". But my code asked isinstance(v, datetime). For a string, that’s always False. The normalization branch is skipped, and then Pydantic parses the string into a tz-aware datetime that gets stored — with the timezone never stripped.

The validator was there. It did nothing. A silent no-op.

Layer 2 — the tests fed a different input than production

So why were the tests green? Because they built the model like this:

# bypasses the conversion path — passes a datetime object directly
req = UploadFinalizeRequest(capture_time=datetime(2024, 5, 2, 3, tzinfo=KST))

Pass an already-built datetime object to the constructor, and the v the before validator sees is a datetime, not a string. Now isinstance(v, datetime) is True — and the validator looks like it works.

The tests fed a different input type than production. Production deserializes a JSON string; the tests assembled an object by hand. Same model, different entry path. So the tests skipped the very conversion I meant to verify (string → parse → tz strip) and lit up green anyway. A false green.

The fix — two lines

# validator: mode="after". Runs after parsing, so v is always a datetime (or None).
@field_validator("capture_time", mode="after")
@classmethod
def _strip_tz(cls, v: datetime | None) -> datetime | None:
    if v is not None and v.tzinfo is not None:
        return v.astimezone(timezone.utc).replace(tzinfo=None)
    return v
# test: same input shape as production (a JSON string).
req = UploadFinalizeRequest.model_validate_json('{"capture_time": "2024-05-02T03:00:00+09:00"}')
assert req.capture_time == datetime(2024, 5, 1, 18, 0, 0)   # naive UTC
assert req.capture_time.tzinfo is None

In mode="after", Pydantic has already parsed the string into a datetime, so v is always a datetime (or None). The tz strip actually applies. And switching the tests to model_validate_json() reproduces the JSON-string input shape — running them through the very conversion the constructor-built object skipped.

Takeaways

Most of the debugging time went not into fixing the code but into telling apart “why do the tests run but production doesn’t?” Two things stick.


Authorship & citation: Written by Ascendy Engineering; quotable with attribution. Found something wrong? Let us know via a GitHub issue.


Tags: pydantic, validation, testing, false-green, backend