Horton

Thread

Commits

  1. Add a test harness for the binary heap code.

  1. [PATCH] Add tests for binaryheap.c

    Aleksander Alekseev <aleksander@timescale.com> — 2025-06-26T15:16:10Z

    Hi,

The proposed patch adds tests for binaryheap.c. This is similar to our
tests for RB-trees in test_rbtree.c.

-- 
Best regards,
Aleksander Alekseev

  2. Re: [PATCH] Add tests for binaryheap.c

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-26T20:46:50Z

    On Thu, Jun 26, 2025 at 06:16:10PM +0300, Aleksander Alekseev wrote:
> The proposed patch adds tests for binaryheap.c. This is similar to our
> tests for RB-trees in test_rbtree.c.

Adding some tests here seems like a good idea.

I'm not sure I see much point in testing both min-heaps and max-heaps.  The
only difference between the two is in the comparator, so IMHO the extra
tests really only serve to test the test comparator.

-- 
nathan

  3. Re: [PATCH] Add tests for binaryheap.c

    Aleksander Alekseev <aleksander@timescale.com> — 2025-06-27T11:02:22Z

    Hi Nathan,

> Adding some tests here seems like a good idea.
>
> I'm not sure I see much point in testing both min-heaps and max-heaps.  The
> only difference between the two is in the comparator, so IMHO the extra
> tests really only serve to test the test comparator.

Make sense. Here is the corrected patch.

-- 
Best regards,
Aleksander Alekseev

  4. Re: [PATCH] Add tests for binaryheap.c

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-16T19:34:13Z

    On Fri, Jun 27, 2025 at 02:02:22PM +0300, Aleksander Alekseev wrote:
>> I'm not sure I see much point in testing both min-heaps and max-heaps.  The
>> only difference between the two is in the comparator, so IMHO the extra
>> tests really only serve to test the test comparator.
> 
> Make sense. Here is the corrected patch.

I trimmed this down some more while trying to keep the coverage the same.

-- 
nathan

  5. Re: [PATCH] Add tests for binaryheap.c

    Aleksander Alekseev <aleksander@tigerdata.com> — 2025-07-17T12:03:55Z

    Hi Nathan,

> > Make sense. Here is the corrected patch.
>
> I trimmed this down some more while trying to keep the coverage the same.

LGTM except for a slight issue with double semicolons at
test_binaryheap.c:160. Here is the updated patch.

  6. Re: [PATCH] Add tests for binaryheap.c

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-17T21:34:16Z

    Committed after some further editorialization.

-- 
nathan