Re: [PATCH] Add tests for Bitmapset

On Sep 24 2025, at 7:28 pm, Michael Paquier <michael@paquier.xyz> wrote:

> On Wed, Sep 24, 2025 at 07:39:59AM -0400, Greg Burd wrote:
>> Thanks Michael, Tom, for the help getting this into shape and in the tree.
> 
> By the way, Greg, do you think that we should aim for a state where we
> are closer to completion?

Hi Michael,

We've come this far, why not go all the way? :)

> We have the module up to the point where we
> are in pretty good shape, with most things and the infrastructure done
> but it can be improved a bit more, as well.

Agreed.

> Based on the information provided by the coverage report at
> https://coverage.postgresql.org/src/backend/nodes/bitmapset.c.gcov.html,
> we still have the following things:
> - bms_equal for different word counts
> - bms_union, bms_nonempty_difference, bms_is_subset and bms_intersect
> with shorter word counts.
> - bms_different with different word counts
> - A couple more cases with bms_subset_compare
> - bms_member_index and word counts
> - bms_overlap_list with negative number in input list.
> - bms_singleton_member ERROR with empty input.
> - bms_get_singleton_member with NULL input
> - bms_del_member with word counts
> - bms_replace_members and repalloc case
> - bms_add_range, bms_join and bms_del_members, more word count cases
> - bms_prev_member and the prevbit business

Thanks for the detailed analysis, I tried to address each of these in
the patch.

> There is not much we can do with the random function, still we could
> do something about the NULL paths in the internal functions:
> https://coverage.postgresql.org/src/test/modules/test_bitmapset/test_bitmapset.c.gcov.html

Agreed.

> The coverage of the latter matters less than the coverage of the
> former, of course.

Agreed.

> --
> Michael


test_bitmapset.c
lines......: 98.9%
functions..: 100%
branches...: 82.6%

Here I removed a bit of code I thought was unnecessary and added a few
test cases.  The coverage is better overall, not much to say about it.


bitmapset.c
lines......: 99.8%
functions..: 100%
branches...: 89.4%

A few branches were really tricky, my (least) favorite was the
bms_subset_compare().  To get to the first BMS_DIFFERENT return you have
to have a test case where the shorter bitmap is long enough to force two
or more iterations of the loop and in the first iteration the first word
in a needs to be a subset of b and then for the second word the opposite
where the second word of b is a subset of a and yet there can be no
overlapping bits in the two sets and the same number of words in both sets.

test_bms_subset_compare('(b 1 64 65)', '(b 1 2 64)') -> BMS_DIFFERENT

Technically the '(b 64 200)', '(b 1 201)' test case is also
BMS_DIFFERENT and so redundant, but I was so happy to find the test case
for this branch I kept them both.

best.

-greg