Feedback-ID: i675e48f3:Fastmail
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: [PATCH] Add tests for Bitmapset
From: "Burd, Greg" <greg@burd.me>
In-Reply-To: <aLpENihhKL4vOxeX@nathan>
Date: Fri, 5 Sep 2025 10:48:21 -0400
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <02DB5E92-1E94-4617-AC11-836486F63BD5@burd.me>
References: <7BD1ABDB-B03A-464A-9BA9-A73B55AD8A1F@getmailspring.com>
 <aLpENihhKL4vOxeX@nathan>
To: Nathan Bossart <nathandbossart@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/02DB5E92-1E94-4617-AC11-836486F63BD5%40burd.me>
Precedence: bulk


> On Sep 4, 2025, at 10:00=E2=80=AFPM, Nathan Bossart =
<nathandbossart@gmail.com> wrote:
>=20
> On Fri, Aug 15, 2025 at 11:39:23AM -0400, Greg Burd wrote:
>> I noticed that there are no tests for Bitmapset in src/test/modules =
as
>> is the case for other similar things like radixtree, rbtree, etc. so =
I
>> created one.  I realize that Bitmapset is already "tested" by all the
>> other code that uses it, but I was able to find one minor =
oversight[1]
>> in that code with these new tests.
>>=20
>> I hope I've covered all the bases, but if you have thoughts on other
>> ways to test Bitmapset I'll happily add them to the patch.
>=20
> Adding some tests here seems like a good idea.  I might look into some =
ways
> to trim it down a bit, but that'd just be minor editorialization.  One
> other thing to consider is adding randomness to the tests (see
> test_radixtree and test_binaryheap for examples).

Nathan,

Thank you for your interest in this patch, I appreciate that your time =
is
limited and highly valuable to the community.  This patch isn't "earth
shattering", but I think it's valuable to have test coverage even in =
cases
where the code being tested is already very well exercised.

I looked at both radix tree and binary heap and how they use random sets =
when
testing.  Binary heap uses it to create different random sets of numbers =
to
use across multiple tests while radix tree has a single function that =
focuses
on randomized data.  I decided not to add randomization into the tests =
of
Bitmapset simply because I like avoiding non-deterministic behavior.  =
But in
tests I guess that can be helpful finding future unknown corner cases.  =
I'm
on the fence as to the value, your call.  :)

Let me know if you'd like that or not.

>=20
> --=20
> Nathan

best, and thanks again for the attention,

-greg