test_fixup_256 builds a Node256 whose key is the end of a range write, but every child is written at the same version, so max(childMaxVersion[0..15]) and max(maxOfMax) agree and the buggy childMaxVersion scan cannot be distinguished from the fixed maxOfMax scan. Per review, that test is removed rather than kept with an explanatory comment; it is superseded by the test below, which exercises the same shape with the versions that actually distinguish the two implementations.
The replacement, test_fixup_256_heterogeneous_versions, uses the minimal reproducer from the issue:
a Node256 under "\x10" with all 256 children at version 1,
a high-version write to the child with index 0x50 (>= 16) at version 10,
a range write [0x05, 0x10) at version 11 that ends at the Node256, so eraseInRange -> fixupMaxVersion recomputes its maxVersion,
then a read at version 5 of "\x10\x50", which must report Conflict.
DebugConflictSet bisimulates against the skip list, so the test checks the result, not just that it doesn't crash.
Verified: with ConflictSet.cpp:2928 reverted to self256->childMaxVersion the new test fails (AssertionError in DebugConflictSet.check: radix tree says Commit, skip list says Conflict); with the fix it passes. All 16 script_test_* tests (32 ctest entries including the valgrind variants) pass.
Closes #89
`test_fixup_256` builds a `Node256` whose key is the end of a range write, but every child is written at the *same* version, so `max(childMaxVersion[0..15])` and `max(maxOfMax)` agree and the buggy `childMaxVersion` scan cannot be distinguished from the fixed `maxOfMax` scan. Per review, that test is removed rather than kept with an explanatory comment; it is superseded by the test below, which exercises the same shape with the versions that actually distinguish the two implementations.
The replacement, `test_fixup_256_heterogeneous_versions`, uses the minimal reproducer from the issue:
- a `Node256` under `"\x10"` with all 256 children at version 1,
- a high-version write to the child with index `0x50` (>= 16) at version 10,
- a range write `[0x05, 0x10)` at version 11 that ends at the `Node256`, so `eraseInRange` -> `fixupMaxVersion` recomputes its maxVersion,
- then a read at version 5 of `"\x10\x50"`, which must report `Conflict`.
`DebugConflictSet` bisimulates against the skip list, so the test checks the result, not just that it doesn't crash.
Verified: with `ConflictSet.cpp:2928` reverted to `self256->childMaxVersion` the new test fails (`AssertionError` in `DebugConflictSet.check`: radix tree says `Commit`, skip list says `Conflict`); with the fix it passes. All 16 `script_test_*` tests (32 ctest entries including the valgrind variants) pass.
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m53s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (pull_request) Successful in 3m51s
CI / test (arm64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 22, https://minio.weaselab.dev/public/aarch64/msan-toolchain-22.1.8.tar.zst, debug-arm64, ubuntu-latest-arm64) (pull_request) Successful in 3m43s
CI / test (amd64, -DCMAKE_BUILD_TYPE=Debug -DMSAN_TOOLCHAIN_PATH=/opt/msan, 21, https://minio.weaselab.dev/public/x86_64/msan-toolchain-21.1.8.tar.zst, debug, ubuntu-latest-amd64) (pull_request) Successful in 4m12s
Every child of the Node256 in test_fixup_256 is written at the same
version, so max(childMaxVersion[0..15]) and max(maxOfMax) agree and the
buggy code passes. test_fixup_256_heterogeneous_versions covers the same
Node256-under-a-range-end shape with distinct versions, so keep only that.
weaselbot
changed title from Add regression test for Node256 fixupMaxVersion false-Commit bug to Replace test_fixup_256 with a regression test for the Node256 fixupMaxVersion false-Commit bug2026-10-27 18:10:56 +00:00
Done in c94e9aa: test_fixup_256 is removed outright. test_fixup_256_heterogeneous_versions supersedes it (same Node256-at-the-end-of-a-range-write shape, but with the high-version child at index 0x50), and it fails on the reverted childMaxVersion line.
Done in c94e9aa: `test_fixup_256` is removed outright. `test_fixup_256_heterogeneous_versions` supersedes it (same `Node256`-at-the-end-of-a-range-write shape, but with the high-version child at index `0x50`), and it fails on the reverted `childMaxVersion` line.
andrew
merged commit b1c1f8f2ee into main2026-10-27 19:33:19 +00:00
andrew
deleted branch weaselbot/issue-892026-10-27 19:33:22 +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.
andrew
Closes #89
test_fixup_256builds aNode256whose key is the end of a range write, but every child is written at the same version, somax(childMaxVersion[0..15])andmax(maxOfMax)agree and the buggychildMaxVersionscan cannot be distinguished from the fixedmaxOfMaxscan. Per review, that test is removed rather than kept with an explanatory comment; it is superseded by the test below, which exercises the same shape with the versions that actually distinguish the two implementations.The replacement,
test_fixup_256_heterogeneous_versions, uses the minimal reproducer from the issue:Node256under"\x10"with all 256 children at version 1,0x50(>= 16) at version 10,[0x05, 0x10)at version 11 that ends at theNode256, soeraseInRange->fixupMaxVersionrecomputes its maxVersion,"\x10\x50", which must reportConflict.DebugConflictSetbisimulates against the skip list, so the test checks the result, not just that it doesn't crash.Verified: with
ConflictSet.cpp:2928reverted toself256->childMaxVersionthe new test fails (AssertionErrorinDebugConflictSet.check: radix tree saysCommit, skip list saysConflict); with the fix it passes. All 16script_test_*tests (32 ctest entries including the valgrind variants) pass.@@ -223,6 +223,9 @@ def test_merge_child_node48():def test_fixup_256():# All children are written at the same version, so this test passes evenJust remove this test
Add regression test for Node256 fixupMaxVersion false-Commit bugto Replace test_fixup_256 with a regression test for the Node256 fixupMaxVersion false-Commit bugDone in
c94e9aa:test_fixup_256is removed outright.test_fixup_256_heterogeneous_versionssupersedes it (sameNode256-at-the-end-of-a-range-write shape, but with the high-version child at index0x50), and it fails on the revertedchildMaxVersionline.