Re: [GSoC][PATCH v4 0/7] t: port reftable/pq_test.c to the unit testing framework

[Junio C Hamano <gitster@pobox.com>]

Chandra Pratap <chandrapratap3519@gmail.com> writes:

> In the recent codebase update (commit 8bf6fbd, 2023-12-09), a new unit
> testing framework written entirely in C was introduced to the Git project
> aimed at simplifying testing and reducing test run times.
> Currently, tests for the reftable refs-backend are performed by a custom
> testing framework defined by reftable/test_framework.{c, h}. Port
> reftable/pq_test.c to the unit testing framework and improve upon
> the ported test.
>
> The first two patches in the series are preparatory cleanup, the third patch
> moves the test to the unit testing framework, and the rest of the patches
> improve upon the ported test.
>
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
>
> ---
> Changes in v4:
> - Add a commit message for the second patch
>
> CI/PR for v4: https://github.com/gitgitgadget/git/pull/1745

Hmph.

A larger question is how pristine we want to keep the "imported
source" that is reftable/* stuff.

Obviously we are getting rid of some parts of it (like pq_test.c
here) so our "fork" will no longer be usable, without pulling some
of the stuff our project offers from outside that directory, by
outside people.  This concern is shared with other topics we saw
recently to move the unit tests bundled with the reftable library to
t/unit-tests/.

But this series brings in another twist that makes it a larger
question.  If we are willing to butcher our copy of reftable library
to a shape that cannot be reused by outside folks, do we still want
to use reftable/pq.c instead of getting rid of it and use the other
prio queue implementation we use elsewhere in our system?  If that
will be the longer term direction we want to go, then porting the
unit tests for reftable/pq.c may end up being a wasted effort.

So, I dunno.

> Chandra Pratap(7):
> reftable: remove unncessary curly braces in reftable/pq.c
> reftable: change the type of array indices to 'size_t' in
> t: move reftable/pq_test.c to the unit testing framework
> t-reftable-pq: make merged_iter_pqueue_check() static
> t-reftable-pq: make merged_iter_pqueue_check() callable
> t-reftable-pq: add test for index based comparison
> t-reftable-pq: add tests for merged_iter_pqueue_top()
>
> Makefile                     |   2 +-
> reftable/pq.c                |  29 +++--------
> reftable/pq.h                |   1 -
> reftable/pq_test.c           |  74 ----------------------------
> t/helper/test-reftable.c     |   1 -
> t/unit-tests/t-reftable-pq.c | 155 +++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 166 insertions(+), 96 deletions(-)
>
> Range-diff against v3:
> 1:  3c333e7770 ! 1:  1873fb02ce reftable: change the type of array indices to 'size_t' in reftable/pq.c
>     @@ Metadata
>       ## Commit message ##
>          reftable: change the type of array indices to 'size_t' in reftable/pq.c
>      
>     +    The variables 'i', 'j', 'k' and 'min' are used as indices for
>     +    'pq->heap', which is an array. Additionally, 'pq->len' is of
>     +    type 'size_t' and is often used to assign values to these
>     +    variables. Hence, change the type of these variables from 'int'
>     +    to 'size_t'.
>     +
>          Mentored-by: Patrick Steinhardt <ps@pks.im>
>          Mentored-by: Christian Couder <chriscool@tuxfamily.org>
>          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> 2:  bf547f705a = 2:  3cccf8266a t: move reftable/pq_test.c to the unit testing framework
> 3:  7dd3a2b27f = 3:  4b63849694 t-reftable-pq: make merged_iter_pqueue_check() static
> 4:  c803e7adfc = 4:  3698a7189f t-reftable-pq: make merged_iter_pqueue_check() callable by reference
> 5:  0b03f3567d = 5:  d58c8f709e t-reftable-pq: add test for index based comparison
> 6:  0cdfa6221e = 6:  69521f0ff7 t-reftable-pq: add tests for merged_iter_pqueue_top()