The same rule, written twice. Only one of them can be tested without waiting.
# Reaches out and takes the time
def is_expired(issued_at, ttl_seconds):
return time.time() - issued_at >= ttl_seconds
# Asks for it, with a sensible default
def is_expired(issued_at, ttl_seconds, now=None):
if now is None:
now = time.time
return now() - issued_at >= ttl_seconds
# The test that the first version cannot have
assert is_expired(1000, 60, lambda: 1060) is True
- The parameter is the seam
The first version has one caller in production and no way in from a test. The second has exactly the same production behaviour — nobody passes now — and a way in that costs one argument. A seam like this is what makes the difference between a function that is tested and one that everyone assumes is fine.
- The default belongs in the body, not the signature — usually
now=time.time in the signature actually works, because it stores the function object and calls it fresh each time. now=time.time() would be a disaster: the default is evaluated once, at import, so every token would be judged against the moment the module loaded. The None-and-fill pattern is habit for exactly that reason — it is never wrong, and the shortcut is only sometimes right.
- Read once, decide once
now() - issued_at >= ttl_seconds reads the clock a single time. A version that calls now() in two places can read 1059 and then 1061 either side of a boundary and produce an answer that matches neither reading. Whenever a value can change between two reads, take a copy and work from it.
Two lines of work: fill in the default when now is None, then compare the elapsed time against the lifetime. Read the contract on which side of the boundary counts as expired.
Validate ttl_seconds first — negative is a ValueError. Then if now is None, set now = time.time (the function itself, without calling it). Return now() - issued_at >= ttl_seconds.
Store the reading in a local variable — elapsed = now() - issued_at — so there is exactly one call to now() no matter how the comparison grows later. Then return elapsed >= ttl_seconds.
def is_expired(issued_at, ttl_seconds, now=None): if ttl_seconds < ____: raise ____(f’ttl_seconds cannot be negative, got {ttl_seconds}') if now is ____: now = time.____ elapsed = ____() - issued_at return elapsed ____ ttl_seconds
import time
def is_expired(issued_at, ttl_seconds, now=None):
if ttl_seconds < 0:
raise ValueError(
f"ttl_seconds cannot be negative, got {ttl_seconds}"
)
if now is None:
now = time.time
elapsed = now() - issued_at
return elapsed >= ttl_seconds
Nine of these cases are impossible against a function that calls time.time() itself — you would be reduced to freezegun, unittest.mock.patch, or sleeping. Those tools exist because so much code is written the other way; they are a cure, and the parameter is prevention. When something is hard to test, the shape of the code is usually the reason, not the test.
Binding now() to a local is not about readability. It makes 'read the clock once' a property of the code rather than a thing the current author happened to do — the next person can add a second comparison without accidentally taking a second reading.
A negative ttl is rejected, but a clock that reads backwards is not. The difference is who made the mistake: a negative lifetime can only come from broken arithmetic in the caller, while a backwards clock is the world being the world. Raise at bugs, absorb reality.
Do not thread a clock parameter through ten layers to reach the one function that needs it — at that point the dependency belongs on an object, or the decision belongs further out where the time is already known. And in a codebase that already uses freezegun or time-machine consistently, adding a second mechanism for the same job makes tests harder to read, not easier. The parameter is right when the function is the boundary; a shared clock object is right when a whole subsystem needs one.
- Add a leeway argument so a token within a few seconds of expiry is still accepted, and decide whether the leeway extends life or shortens it — the sign is easy to get backwards.
- Write expires_in(issued_at, ttl_seconds, now=None) returning the seconds remaining, and make is_expired call it. Watch what it should return once the token is already dead.
- Do the same to a function that uses random: take the generator as an argument, and see how the tests you can now write compare to seeding a global.