How to Effectively Present a Bugfix in Two Steps Using Unit Tests
The Problem
Consider this simple Python function:
def divide(a, b):
return a / b
And a unit test that covers it:
def test_divide():
assert divide(4, 1) == 4
assert divide(4, 2) == 2
assert divide(4, 4) == 1
At first glance, most developers will notice a glaring issue with the divide function:
>>> divide(4, 0)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/thomas/divide.py", line 5, in divide
return a / b
~~^~~
ZeroDivisionError: division by zero
As it stands, the function will raise a ZeroDivisionError and crash the program if b is zero. Clearly, this behavior needs to be handled.
The Classic Fix
To fix the bug, we decide that the function should return None if there is an attempt to divide by zero:
def divide(a, b):
if b == 0:
return None
return a / b
Now, we update our unit test to reflect this new behavior:
def test_divide():
assert divide(4, 1) == 4
assert divide(4, 2) == 2
assert divide(4, 4) == 1
assert divide(4, 0) is None
With the fix in place, we can commit the changes, push them for review, and consider the issue resolved.
And while that’s technically correct, there’s a more structured way to present this change to your team that makes reviewing easier.
A More Structured Approach: Two-Step Bug Fix
In more complex cases, it’s not always immediately obvious to reviewers whether the bug was really there. When code and tests are modified together in the same commit, it can be hard to pinpoint the original problem. Was the issue truly with a ZeroDivisionError or something else? Without seeing the error firsthand, reviewers might miss critical context.
A clearer approach is to present the fix in two separate commits: one that exposes the bug, and another that implements the fix. This method allows for a more transparent understanding of the problem and how it was resolved.
Step 1: Expose the Bug in the Test
First, we leave the buggy code as-is but update the test to reveal the issue:
def divide(a, b):
return a / b
And here’s the updated test, using pytest to ensure that dividing by zero raises an error:
import pytest
def test_divide():
assert divide(4, 1) == 4
assert divide(4, 2) == 2
assert divide(4, 4) == 1
with pytest.raises(ZeroDivisionError):
divide(4, 0)
This first commit allows reviewers to clearly see the problem without any confusion. It also ensures that the CI won’t break, since the test expects the error.
Step 2: Implement the Fix
In the next commit, we apply the bugfix and adjust the test accordingly. The final diff looks like this:
def divide(a, b):
+ if b == 0:
+ return None
return a / b
-import pytest
-
def test_divide():
assert divide(4, 1) == 4
assert divide(4, 2) == 2
assert divide(4, 4) == 1
- with pytest.raises(ZeroDivisionError):
- divide(4, 0)
+ assert divide(4, 0) is None
Now, the change is easy to follow: the test initially revealed the problem, and the second commit resolved it. This two-step process makes it much easier for reviewers to understand both the bug and the fix.
By splitting the bugfix into two commits, you make it easier for reviewers to grasp the original problem and evaluate the effectiveness of the solution. This structured approach not only improves the clarity of your pull request but also enhances the overall review process.
By Thomas Martin
Follow me or comment