You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A rollback bug shipped in Gemini CLI last month under a green rollback test. I sent the fix today, and the shape of the failure seems worth writing down, because it isn't specific to that codebase and it's directly relevant to what this repo is about.
The code created a temp dir, ran the update, and on failure copied the temp dir back over the extension. Nothing ever copied the extension into the temp dir. The temp dir was always empty, so the rollback restored nothing.
There was a test for exactly this path. It passed. Its entire rollback assertion was:
copyExtension was mocked as a bare vi.fn(), and the temp dir was stubbed to a literal string. So the test asserted the restore call was made with the expected arguments. That's equally true when the directory is empty. It checked the call, not the effect, and it passed identically in both worlds.
The asymmetry is the part I'd generalize. There was an assertion for the restore direction and none for the backup direction, and the missing call was precisely the unasserted one. Not a coincidence: a mocked collaborator can only witness the calls someone thought to assert, so the blind spot and the bug are drawn from the same set.
Two things I'd take from it:
For an undo path, assert an effect, not a call. Write a real file, force the failure, read the contents back. If restoring becomes a no-op again, only that version fails.
When you assert one direction of a paired operation, check whether the other direction is asserted anywhere. If it isn't, that's where to look first.
The second one is cheap enough to run over an existing suite by hand, and I found it more productive than adding tests.
Has anyone found a linter or a review heuristic that catches unpaired assertions like this? I've only done it by reading, and I'd rather it were mechanical.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
A rollback bug shipped in Gemini CLI last month under a green rollback test. I sent the fix today, and the shape of the failure seems worth writing down, because it isn't specific to that codebase and it's directly relevant to what this repo is about.
The code created a temp dir, ran the update, and on failure copied the temp dir back over the extension. Nothing ever copied the extension into the temp dir. The temp dir was always empty, so the rollback restored nothing.
There was a test for exactly this path. It passed. Its entire rollback assertion was:
copyExtensionwas mocked as a barevi.fn(), and the temp dir was stubbed to a literal string. So the test asserted the restore call was made with the expected arguments. That's equally true when the directory is empty. It checked the call, not the effect, and it passed identically in both worlds.The asymmetry is the part I'd generalize. There was an assertion for the restore direction and none for the backup direction, and the missing call was precisely the unasserted one. Not a coincidence: a mocked collaborator can only witness the calls someone thought to assert, so the blind spot and the bug are drawn from the same set.
Two things I'd take from it:
The second one is cheap enough to run over an existing suite by hand, and I found it more productive than adding tests.
PR: google-gemini/gemini-cli#29166
Issue: google-gemini/gemini-cli#29033
Has anyone found a linter or a review heuristic that catches unpaired assertions like this? I've only done it by reading, and I'd rather it were mechanical.
All reactions