public inbox for [email protected]  

help / color / mirror / Atom feed

From: Burd, Greg <[email protected]>
To: Nathan Bossart <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Add tests for Bitmapset
Date: Fri, 5 Sep 2025 10:48:21 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <aLpENihhKL4vOxeX@nathan>
References: <[email protected]>
	<aLpENihhKL4vOxeX@nathan>


> On Sep 4, 2025, at 10:00 PM, Nathan Bossart <[email protected]> wrote:
> 
> 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.
>> 
>> 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.
> 
> 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.

> 
> -- 
> Nathan

best, and thanks again for the attention,

-greg

view thread (81+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Add tests for Bitmapset
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox