Skip to content

unit/btree: test basic B-Tree operations#18690

Open
chrislongros wants to merge 1 commit into
openzfs:masterfrom
chrislongros:unit/btree-tests
Open

unit/btree: test basic B-Tree operations#18690
chrislongros wants to merge 1 commit into
openzfs:masterfrom
chrislongros:unit/btree-tests

Conversation

@chrislongros

@chrislongros chrislongros commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

module/zfs/btree.c has no dedicated unit test. This adds a small tests/unit suite for its basic public operations.

Description

Adds tests/unit/test_btree.c with four tests over a tree of uint64_t values:

  • empty — a new tree has no elements and nothing to walk
  • add_find — added values are found and the count tracks them; a value never added is not present
  • remove — removing a value drops it and lowers the count, leaving the rest intact
  • walk — values inserted out of order are returned smallest-to-largest via zfs_btree_first/zfs_btree_next

How Has This Been Tested?

make unit T=btree on Linux x86_64:

Running test suite with seed 0xb6f08df4...
btree.empty                          [ OK    ] [ 0.00000293 / 0.00000176 CPU ]
btree.add_find                       [ OK    ] [ 0.00001506 / 0.00001447 CPU ]
btree.remove                         [ OK    ] [ 0.00001397 / 0.00001336 CPU ]
btree.walk                           [ OK    ] [ 0.00000735 / 0.00000700 CPU ]
4 of 4 (100%) tests successful, 0 (0%) test skipped.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist

  • My code follows the OpenZFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

Cover an empty tree, add/find, remove, and walk over a tree of uint64_t
values.

Signed-off-by: Christos Longros <chris.longros@gmail.com>
Comment thread tests/unit/test_btree.c
return (-1);
if (x > y)
return (1);
return (0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the TREE_CMP macro here.

Suggested change
return (0);
return (TREE_CMP(x, y));

Comment thread tests/unit/test_btree.c
UNIT_TEST("empty", test_btree_empty),
UNIT_TEST("add_find", test_btree_add_find),
UNIT_TEST("remove", test_btree_remove),
UNIT_TEST("walk", test_btree_walk),

@behlendorf behlendorf Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's adapt the find_without_index() and drain_tree() tests from ./tests/zfs-tests/cmd/btree_test.c and move them to this unit test. I'd like to also move stress_tree() in to the unit tests, but since it depends on an AVL tree maybe it belongs in its own test_btree_stress.c. I'm open to other suggestions, it'd just be nice to remove it from the ZTS tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we want test_avl before or at the same time, and then some sort of correctness test that proves that they do the same thing.

Semi-related, I'm not entirely certain where stress/performance tests fit in all this (which is part of why I haven't just dragged the ICP/algorithm tests into this framework yet). I couldn't quickly find good literature on how they fit into an overall testing position, and haven't had much chance to think about it. I'm not saying "don't do it" (better to do something than nothing) and I don't think ZTS is the right place for some of those either. So probably just do it and we can move it around later if we think of something better. But I hope someone that actually knows testing theory and practice in more depth can just say "oh yeah, you want Florple Tests, here's the prior art, wire it this way". I always hope for that sort of thing! 😆

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jun 22, 2026
@robn robn assigned robn and unassigned robn Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants