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: <aLsvPdSHJdRCEk7v@nathan>
Date: Mon, 8 Sep 2025 14:21:28 -0400
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <9E1E0BA4-952C-412A-884A-6E700F26B0CA@burd.me>
References: <7BD1ABDB-B03A-464A-9BA9-A73B55AD8A1F@getmailspring.com>
 <aLpENihhKL4vOxeX@nathan> <02DB5E92-1E94-4617-AC11-836486F63BD5@burd.me>
 <aLsvPdSHJdRCEk7v@nathan>
To: Nathan Bossart <nathandbossart@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/9E1E0BA4-952C-412A-884A-6E700F26B0CA%40burd.me>
Precedence: bulk


> On Sep 5, 2025, at 2:43=E2=80=AFPM, Nathan Bossart =
<nathandbossart@gmail.com> wrote:
>=20
> On Fri, Sep 05, 2025 at 10:48:21AM -0400, Burd, Greg wrote:
>> 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.  :)
>=20
> I'm not too concerned about it.  We've lived without a dedicated test =
suite
> for Bitmapset for a very long time, so any amount of test coverage is =
an
> improvement.  Like you said, adding some randomization might be =
helpful for
> finding weird bugs we wouldn't have thought to test.  And, given the =
many,
> many machines that run the tests, IMHO it'd only help build even more
> confidence in the code.  If my suggestion inspires you to update the =
patch,
> great, but I'm fine with proceeding with what you already wrote, too.

Nathan, thanks for considering the patch.  Honestly, I'm fine with it as =
is.
We can revisit later if needed.  This does what I'd intended, test and =
document
in code the API and implementation making future changes to that more
transparent.

Thanks!

-greg

--=20
> nathan