[GSoC,v2,0/6] t: port reftable/pq_test.c to the unit testing

Message

Chandra Pratap June 6, 2024, 3:23 p.m. UTC

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 patch in the series is preparatory cleanup, the second patch
moves the test to the unit testing framework, and the rest of the patches
improve upon the ported test.

Mentored-by: Patrick Steinhardt <[e-mail]>
Mentored-by: Christian Couder <[e-mail]>
Signed-off-by: Chandra Pratap <[e-mail]>

---
Changes in v2:
- Change data type of index variable 'i' to 'size_t' from 'int'
- Use correct format specifier for 'size_t' types
- Improve the test for merged_iter_pqueue_top() by asserting equality
  between 'top.rec' and the corresponding record in 'recs' array

CI/PR for v2: https://github.com/gitgitgadget/git/pull/1745

Chandra Pratap (6):
reftable: clean up reftable/pq.c
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                |  18 +++--------
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, 160 insertions(+), 91 deletions(-)


Range-diff against v1:
1:  f18b610f63 ! 1:  f24dc84877 t: move reftable/pq_test.c to the unit testing framework
    @@ t/unit-tests/t-reftable-pq.c: license that can be found in the LICENSE file or a
      {
     -	int i;
     -	for (i = 1; i < pq.len; i++) {
    -+	for (int i = 1; i < pq.len; i++) {
    - 		int parent = (i - 1) / 2;
    +-		int parent = (i - 1) / 2;
     -
     -		EXPECT(pq_less(&pq.heap[parent], &pq.heap[i]));
    ++	for (size_t i = 1; i < pq.len; i++) {
    ++		size_t parent = (i - 1) / 2;
     +		check(pq_less(&pq.heap[parent], &pq.heap[i]));
      	}
      }
    @@ t/unit-tests/t-reftable-pq.c: license that can be found in the LICENSE file or a
      	for (i = 0; i < N; i++) {
      		struct strbuf refname = STRBUF_INIT;
     -		strbuf_addf(&refname, "%02d", i);
    -+		strbuf_addf(&refname, "%02ld", (long)i);
    ++		strbuf_addf(&refname, "%02"PRIuMAX, (uintmax_t)i);

      		reftable_record_init(&recs[i], BLOCK_TYPE_REF);
      		recs[i].u.ref.refname = strbuf_detach(&refname, NULL);
2:  798e0adfeb ! 2:  ce42fd1288 t-reftable-pq: make merged_iter_pqueue_check() static
    @@ t/unit-tests/t-reftable-pq.c: license that can be found in the LICENSE file or a
     -void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
     +static void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
      {
    - 	for (int i = 1; i < pq.len; i++) {
    - 		int parent = (i - 1) / 2;
    + 	for (size_t i = 1; i < pq.len; i++) {
    + 		size_t parent = (i - 1) / 2;
3:  8412672105 ! 3:  226d72aa6a t-reftable-pq: make merged_iter_pqueue_check() callable by reference
    @@ t/unit-tests/t-reftable-pq.c: license that can be found in the LICENSE file or a
     -static void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
     +static void merged_iter_pqueue_check(const struct merged_iter_pqueue *pq)
      {
    --	for (int i = 1; i < pq.len; i++) {
    -+	for (int i = 1; i < pq->len; i++) {
    - 		int parent = (i - 1) / 2;
    +-	for (size_t i = 1; i < pq.len; i++) {
    ++	for (size_t i = 1; i < pq->len; i++) {
    + 		size_t parent = (i - 1) / 2;
     -		check(pq_less(&pq.heap[parent], &pq.heap[i]));
     +		check(pq_less(&pq->heap[parent], &pq->heap[i]));
      	}
4:  2ef5b8b0b1 ! 4:  00cb440f11 t-reftable-pq: add test for index based comparison
    @@ t/unit-tests/t-reftable-pq.c: static void test_pq(void)
      	merged_iter_pqueue_release(&pq);
      }

    -+
     +static void test_pq_index(void)
     +{
     +	struct merged_iter_pqueue pq = { 0 };
5:  838e67d2a3 ! 5:  dd44486c28 t-reftable-pq: add tests for merged_iter_pqueue_top()
    @@ t/unit-tests/t-reftable-pq.c: static void test_pq_record(void)
      		check(reftable_record_type(e.rec) == BLOCK_TYPE_REF);
      		if (last)
      			check_int(strcmp(last, e.rec->u.ref.refname), <, 0);
    -@@ t/unit-tests/t-reftable-pq.c: static void test_pq_record(void)
    - 	merged_iter_pqueue_release(&pq);
    - }
    -
    --
    - static void test_pq_index(void)
    - {
    - 	struct merged_iter_pqueue pq = { 0 };
     @@ t/unit-tests/t-reftable-pq.c: static void test_pq_index(void)
      	}

    @@ t/unit-tests/t-reftable-pq.c: static void test_pq_index(void)
     +		merged_iter_pqueue_check(&pq);
     +	}
     +
    -+	while (!merged_iter_pqueue_is_empty(pq)) {
    ++	for (i = N - 1; !merged_iter_pqueue_is_empty(pq); i--) {
     +		struct pq_entry top = merged_iter_pqueue_top(pq);
     +		struct pq_entry e = merged_iter_pqueue_remove(&pq);
     +
     +		merged_iter_pqueue_check(&pq);
     +		check(pq_entry_equal(&top, &e));
    -+		for (i = 0; i < pq.len; i++) {
    -+			check(pq_less(&top, &pq.heap[i]));
    -+			check_int(top.index, >, i);
    ++		check(reftable_record_equal(top.rec, &recs[i], GIT_SHA1_RAWSZ));
    ++		for (size_t j = 0; i < pq.len; j++) {
    ++			check(pq_less(&top, &pq.heap[j]));
    ++			check_int(top.index, >, j);
     +		}
     +	}
     +