My CI went red on a pull request that was, as far as I could tell, correct.
The change was small and boring: an OAuth helper in a CLI tool used to discover a free port by binding it, reading the number, and closing the socket — which leaves a window in which any other process can take the port before the callback server binds it. The fix binds once and hands the already-bound socket to the HTTP server, so there is no window. I measured it on my machine: with the old helper another process could take the port; with the new one it could not. I added a test, ran it, and pushed.
Then CI came back: Test 3.12, 3.13, Canary — all failed, on the Linux legs, while the identical suite was green on Windows on the same commit. The one failing test was the one I had just written:
OSError: [Errno 98] Address already in use
809 passed, 1 failed
The fix was fine. The test I shipped with the fix was not.
The test that broke
The test asked a simple question: after the OAuth flow finishes, is the redirect port released? And it answered it in the most obvious way — by trying to bind the port again:
def _port_is_free(port: int) -> bool:
with socket.socket() as s:
try:
s.bind(("127.0.0.1", port))
return True
except OSError:
return False
If a fresh listener can bind, the port is free. That is the intuitive phrasing. It is also the wrong instrument, and the reason is entirely about which states of a TCP connection a rebind is willing to forgive.
Attempt one: blaming TIME_WAIT
My first hypothesis was the classic one: the server has closed, but the connection sits in TIME_WAIT, and TIME_WAIT blocks a rebind. The standard fix for that is SO_REUSEADDR, so I set it on the probe socket and pushed again.
CI went red again — with a new signal that killed the hypothesis. The probe now returned False even with SO_REUSEADDR set. If TIME_WAIT were the cause, SO_REUSEADDR is exactly what would have forgiven it. It didn't, so the port was not merely in TIME_WAIT.
The tell was sitting right next to it. There were two tests: one where the flow completes (the failing one) and one where it does not. The sibling, which also rebinds the port, passed. The only difference between them was that in the failing test a client had actually connected and received a response. That points at a lingering accepted connection — a socket in FIN_WAIT_2 or ESTABLISHED — and SO_REUSEADDR deliberately does not forgive those. It forgives TIME_WAIT because a timed-out connection is genuinely gone from the application's point of view; it does not forgive a live connection because that would let two listeners fight over the same 4-tuple.
So the failure had two independent halves:
- the probe used an instrument (
bind) that is sensitive to connection state I did not control; - the test's fake client never closed its response, so a connection really was left hanging — a genuine leak in the test itself, the same kind of leak the production fix was about.
Attempt two: change the question, not the port
The fix was to stop asking "can something bind here" and start asking "is anything still listening here":
def _nothing_is_listening(port: int) -> bool:
with socket.socket() as s:
try:
s.connect(("127.0.0.1", port))
return False # something answered
except (ConnectionRefusedError, OSError):
return True # nothing there
A connect() that is refused tells you there is no listener — and it does not care what state some old connection is in, because it never touches that connection's 4-tuple. That is the property I actually wanted to assert: "the server is gone", not "the kernel will let me rebind". I also closed the response in the fake client, removing the leak underneath.
CI returned 5/5 green.
Proving the probe isn't vacuous
A negative assertion has a failure mode that a positive one does not: it can be vacuously true. A probe that always says "nothing is listening" would pass the test perfectly and tell you nothing. So I added a control arm that runs the same probe against three states:
| state | _nothing_is_listening |
|---|---|
| a listener is up | False |
| listener just closed, cleanly | True |
| a connection lingers, no listener | True |
The first row is the one that matters: the probe can return False, so the True in the test means something. Without that row, "nothing is listening" is an unfalsifiable claim.
What generalises
Three things, and none of them are about OAuth or Python.
A fix's own test is part of the fix. The pull request was not "the socket handoff"; it was "the socket handoff and a test that verifies it on every platform the suite runs on". A green local run on one OS is a sample, not a verdict — and when a CI leg fails where you did not look, the bug is often in the instrument, not the code.
For a negative assertion, choose an instrument that is insensitive to the state you do not control. "The port is released" is a claim about the server being gone. bind-succeeds smuggles in an assumption about kernel connection bookkeeping; connect-refused does not. Pick the phrasing that says only what you mean.
Give every negative probe a control arm. A probe that cannot fail is not evidence. One extra case — the state in which it must return the other answer — is what turns "the test passed" into "the property held".
The second attempt was smaller than the first and took less time to write. The expensive part was not the fix; it was noticing that the instrument, not the code, was the thing that needed changing.
Top comments (0)