The second test even includes a comment explaining that the bug being regression-tested used to produce an incorrect commit. If the radix tree reintroduces that bug, the current test will still pass silently because the [Result.COMMIT] / [Result.CONFLICT] expectation is never checked.
Impact: These tests only verify that the calls do not crash, not that the implementation returns the correct conflict-set result. A regression in the thread-local InternalVersionT::zero handling would go unnoticed.
Expected behavior:
test_update_zero_should_commit should assert cs2.check(read(0, b"\x00", bytes([256 - 17]))) == [Result.COMMIT].
test_update_zero_should_conflict should assert cs1.check(read(2**32 + 1, b"\x00")) == [Result.CONFLICT].
Actual behavior: Both calls return unchecked results, so a regression in either direction passes undetected.
`test_conflict_set.py` has two regression tests named after the result they are meant to verify, but neither asserts the return value of `check()`:
- `test_update_zero_should_commit`, `test_conflict_set.py:71`:
```python
cs2.check(read(0, b"\x00", bytes([256 - 17])))
```
- `test_update_zero_should_conflict`, `test_conflict_set.py:84`:
```python
cs1.check(read(2**32 + 1, b"\x00"))
```
The second test even includes a comment explaining that the bug being regression-tested used to produce an incorrect commit. If the radix tree reintroduces that bug, the current test will still pass silently because the `[Result.COMMIT]` / `[Result.CONFLICT]` expectation is never checked.
**Impact:** These tests only verify that the calls do not crash, not that the implementation returns the correct conflict-set result. A regression in the thread-local `InternalVersionT::zero` handling would go unnoticed.
**Expected behavior:**
- `test_update_zero_should_commit` should assert `cs2.check(read(0, b"\x00", bytes([256 - 17]))) == [Result.COMMIT]`.
- `test_update_zero_should_conflict` should assert `cs1.check(read(2**32 + 1, b"\x00")) == [Result.CONFLICT]`.
**Actual behavior:** Both calls return unchecked results, so a regression in either direction passes undetected.
weaselbot
was assigned by andrew2026-06-21 14:59:28 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
test_conflict_set.pyhas two regression tests named after the result they are meant to verify, but neither asserts the return value ofcheck():test_update_zero_should_commit,test_conflict_set.py:71:test_update_zero_should_conflict,test_conflict_set.py:84:The second test even includes a comment explaining that the bug being regression-tested used to produce an incorrect commit. If the radix tree reintroduces that bug, the current test will still pass silently because the
[Result.COMMIT]/[Result.CONFLICT]expectation is never checked.Impact: These tests only verify that the calls do not crash, not that the implementation returns the correct conflict-set result. A regression in the thread-local
InternalVersionT::zerohandling would go unnoticed.Expected behavior:
test_update_zero_should_commitshould assertcs2.check(read(0, b"\x00", bytes([256 - 17]))) == [Result.COMMIT].test_update_zero_should_conflictshould assertcs1.check(read(2**32 + 1, b"\x00")) == [Result.CONFLICT].Actual behavior: Both calls return unchecked results, so a regression in either direction passes undetected.