Feedback-ID: i675e48f3:Fastmail
Date: Wed, 1 Oct 2025 07:56:07 -0400
From: Greg Burd <greg@burd.me>
To: Michael Paquier <michael@paquier.xyz>, David Rowley
 <dgrowleyml@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Message-ID: <1D456624-470D-43C9-B59D-F487C35E0988@greg.burd.me>
In-Reply-To: <aNy4xV4Fa1m9lyXX@paquier.xyz>
References: <aNy4xV4Fa1m9lyXX@paquier.xyz>
Subject: Re: [PATCH] Add tests for Bitmapset
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/1D456624-470D-43C9-B59D-F487C35E0988%40greg.burd.me>
Precedence: bulk


On Oct 1 2025, at 1:14 am, Michael Paquier <michael@paquier.xyz> wrote:

> On Wed, Oct 01, 2025 at 06:00:59PM +1300, David Rowley wrote:
>> I didn't look at the coverage report, but on thinking about it,
>> test_bitmapset.c's "PG_RETURN_NULL(); /* invalid input */" won't be
>> covered as I stripped out all the tests which purposefully pass
>> invalid input. You may not want it that way if you're keen to keep the
>> coverage of the test module high as well as bitmapset.c
> 
> Yes, that's something I was just going through, with a result pretty
> close to what you have sent, including the removal and replacement of
> most of tests that were specific to the test module.  The prev/next
> member coverage is still looking OK from here, though.
> 
> Thanks for the typos, I've missed these.
> --
> Michael

Michael, David,

Thank you. :)

The coverage[1] looks to be 100% for everything in bitmapset.c now and
98.6% of the test code[2] for Bitmapset -- more than enough and much
better than before.  I do like the cleanup/simplification that you two
jointly applied. I think it's easier to maintain and reason about and
looking at the final result I can see the logic in the simplified
approach.  I've learned a lot from this exercise not only about
Bitmapset and various other areas in the code but also the
approach/model for a task like this.  I'll keep that in mind going forward.

best.

-greg


[1] https://coverage.postgresql.org/src/backend/nodes/bitmapset.c.gcov.html
[2] https://coverage.postgresql.org/src/test/modules/test_bitmapset/test_bitmapset.c.gcov.html