AI reviewer caught missing test coverage #2577
clemenslosbichler-cloud
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
I ran an automated reviewer over a recreation of #2543 and it came back with one
thing I thought was worth passing on.
The PR wraps super().close_connection() in try/finally so the queue-cache and
no-ack cleanup still runs if the base close throws. The two new tests only raise
inside the cleanup loop itself, though, so if you deleted the try/finally the
suite would still pass. The guarantee the PR adds isn't actually pinned by
anything.
It didn't find a bug, to be clear. It found that the test doesn't hold the fix
in place.
Fair warning on how I got this: I forked kombu and recreated #2543 on my copy,
because the thing needs a GitHub App installed to run and I wasn't going to ask
you for that before you'd seen anything. Same diff, same commits.
If you want it pointed at real PRs as they come in I'll set that up, no charge
and no catch. I'm after feedback on how noisy it is, and 74 open PRs is more
signal than I can get anywhere else.
All reactions