← All articles

engineeringreview

What a passing test leaves out

A checkout form can save successfully and still let someone edit a shipped order. The test needs to catch both.

Suppose an agent adds a way to change an order’s delivery address. You try it, reload the page, and the new address is still there. The test passes.

For an order that has already shipped, accepting that edit would be a bug. The package is on its way. A test that only saves a new address won’t catch it.

This is an example, but it gets at a practical problem with reviewing generated code: the implementation and its tests can agree on the wrong behavior.

Test the rule

The rule here is that customers can change an address until the order ships. That gives the reviewer two cases to check.

An unshipped order should accept the new address and keep it after a reload. A shipped order should reject the edit and keep the original address. Checking the saved value matters; an error message alone doesn’t prove the update was refused.

If the agent only tested the first case, the review is unfinished. Asking which case is missing is more useful than asking it to run the same suite again.

Check the test itself

Temporarily remove the guard that rejects edits to shipped orders, then run the rejection test. It should fail because the address changed when it shouldn’t have.

If it still passes, inspect the assertion. Perhaps the test checks that the request returned, but never reads the saved order. Restore the guard after checking; this is a way to examine the test, not a change to ship.

There’s a limit to what this proves. A local test can check this rule while still missing a permissions issue or a database failure. The review should say which behavior was exercised and which questions remain open.

Leave the reason in the change

A short explanation beside the diff helps the next person: address edits are refused after shipment because the parcel has already left. Include the test that checks that restriction.

That gives a future reviewer a reason to preserve the guard. Otherwise, a later agent might see it as an unnecessary condition and remove it while simplifying the code.