diff mbox series

[GSoC,2/6] t: move reftable/pq_test.c to the unit testing framework

Message ID 20240606075601.6989-3-chandrapratap3519@gmail.com (mailing list archive)
State Superseded
Headers show
Series t: port reftable/pq_test.c to the unit testing | expand

Commit Message

Chandra Pratap June 6, 2024, 7:40 a.m. UTC
reftable/pq_test.c exercises a priority queue defined by
reftable/pq.{c, h}. Migrate reftable/pq_test.c to the unit
testing framework. Migration involves refactoring the tests
to use the unit testing framework instead of reftable's test
framework.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
 Makefile                                      |  2 +-
 t/helper/test-reftable.c                      |  1 -
 .../pq_test.c => t/unit-tests/t-reftable-pq.c | 35 ++++++++-----------
 3 files changed, 15 insertions(+), 23 deletions(-)
 rename reftable/pq_test.c => t/unit-tests/t-reftable-pq.c (64%)

Comments

Patrick Steinhardt June 6, 2024, 11:48 a.m. UTC | #1
On Thu, Jun 06, 2024 at 01:10:46PM +0530, Chandra Pratap wrote:
> reftable/pq_test.c exercises a priority queue defined by
> reftable/pq.{c, h}. Migrate reftable/pq_test.c to the unit
> testing framework. Migration involves refactoring the tests
> to use the unit testing framework instead of reftable's test
> framework.
> 
> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
> ---
>  Makefile                                      |  2 +-
>  t/helper/test-reftable.c                      |  1 -
>  .../pq_test.c => t/unit-tests/t-reftable-pq.c | 35 ++++++++-----------
>  3 files changed, 15 insertions(+), 23 deletions(-)
>  rename reftable/pq_test.c => t/unit-tests/t-reftable-pq.c (64%)
> 
> diff --git a/Makefile b/Makefile
> index 59d98ba688..1cabe4cc69 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1336,6 +1336,7 @@ THIRD_PARTY_SOURCES += sha1dc/%
>  UNIT_TEST_PROGRAMS += t-ctype
>  UNIT_TEST_PROGRAMS += t-mem-pool
>  UNIT_TEST_PROGRAMS += t-prio-queue
> +UNIT_TEST_PROGRAMS += t-reftable-pq
>  UNIT_TEST_PROGRAMS += t-strbuf
>  UNIT_TEST_PROGRAMS += t-strcmp-offset
>  UNIT_TEST_PROGRAMS += t-trailer
> @@ -2675,7 +2676,6 @@ REFTABLE_TEST_OBJS += reftable/basics_test.o
>  REFTABLE_TEST_OBJS += reftable/block_test.o
>  REFTABLE_TEST_OBJS += reftable/dump.o
>  REFTABLE_TEST_OBJS += reftable/merged_test.o
> -REFTABLE_TEST_OBJS += reftable/pq_test.o
>  REFTABLE_TEST_OBJS += reftable/record_test.o
>  REFTABLE_TEST_OBJS += reftable/readwrite_test.o
>  REFTABLE_TEST_OBJS += reftable/stack_test.o
> diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
> index bae731669c..86a2b0f91a 100644
> --- a/t/helper/test-reftable.c
> +++ b/t/helper/test-reftable.c
> @@ -9,7 +9,6 @@ int cmd__reftable(int argc, const char **argv)
>  	record_test_main(argc, argv);
>  	block_test_main(argc, argv);
>  	tree_test_main(argc, argv);
> -	pq_test_main(argc, argv);
>  	readwrite_test_main(argc, argv);
>  	merged_test_main(argc, argv);
>  	stack_test_main(argc, argv);
> diff --git a/reftable/pq_test.c b/t/unit-tests/t-reftable-pq.c
> similarity index 64%
> rename from reftable/pq_test.c
> rename to t/unit-tests/t-reftable-pq.c
> index b7d3c80cc7..dcde73de66 100644
> --- a/reftable/pq_test.c
> +++ b/t/unit-tests/t-reftable-pq.c
> @@ -6,35 +6,28 @@ license that can be found in the LICENSE file or at
>  https://developers.google.com/open-source/licenses/bsd
>  */
>  
> -#include "system.h"
> -
> -#include "basics.h"
> -#include "constants.h"
> -#include "pq.h"
> -#include "record.h"
> -#include "reftable-tests.h"
> -#include "test_framework.h"
> +#include "test-lib.h"
> +#include "reftable/constants.h"
> +#include "reftable/pq.h"
>  
>  void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
>  {
> -	int i;
> -	for (i = 1; i < pq.len; i++) {
> +	for (int i = 1; i < pq.len; i++) {

If we're changing this already, then we can also convert it to `size_t
i` while at it to match the type of `pq.len`.

>  		int parent = (i - 1) / 2;
> -
> -		EXPECT(pq_less(&pq.heap[parent], &pq.heap[i]));
> +		check(pq_less(&pq.heap[parent], &pq.heap[i]));
>  	}
>  }
>  
>  static void test_pq(void)
>  {
> -	struct merged_iter_pqueue pq = { NULL };
> +	struct merged_iter_pqueue pq = { 0 };
>  	struct reftable_record recs[54];
> -	int N = ARRAY_SIZE(recs) - 1, i;
> +	size_t N = ARRAY_SIZE(recs) - 1, i;
>  	char *last = NULL;
>  
>  	for (i = 0; i < N; i++) {
>  		struct strbuf refname = STRBUF_INIT;
> -		strbuf_addf(&refname, "%02d", i);
> +		strbuf_addf(&refname, "%02ld", (long)i);

This should rather be:

    strubf_addf(&refname, "%02"PRIuMAX, (uintmax_t) i);

This is splitting hairs though as it does not matter in practice. But
I'd either drop this change to convert `N` and `i` to become a `size_t`,
or I'd do it correctly.

>  		reftable_record_init(&recs[i], BLOCK_TYPE_REF);
>  		recs[i].u.ref.refname = strbuf_detach(&refname, NULL);
> @@ -48,7 +41,6 @@ static void test_pq(void)
>  
>  		merged_iter_pqueue_add(&pq, &e);
>  		merged_iter_pqueue_check(pq);
> -
>  		i = (i * 7) % N;
>  	} while (i != 1);

This removed newline is of dubious value, but I guess that's
subjective.

Patrick
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 59d98ba688..1cabe4cc69 100644
--- a/Makefile
+++ b/Makefile
@@ -1336,6 +1336,7 @@  THIRD_PARTY_SOURCES += sha1dc/%
 UNIT_TEST_PROGRAMS += t-ctype
 UNIT_TEST_PROGRAMS += t-mem-pool
 UNIT_TEST_PROGRAMS += t-prio-queue
+UNIT_TEST_PROGRAMS += t-reftable-pq
 UNIT_TEST_PROGRAMS += t-strbuf
 UNIT_TEST_PROGRAMS += t-strcmp-offset
 UNIT_TEST_PROGRAMS += t-trailer
@@ -2675,7 +2676,6 @@  REFTABLE_TEST_OBJS += reftable/basics_test.o
 REFTABLE_TEST_OBJS += reftable/block_test.o
 REFTABLE_TEST_OBJS += reftable/dump.o
 REFTABLE_TEST_OBJS += reftable/merged_test.o
-REFTABLE_TEST_OBJS += reftable/pq_test.o
 REFTABLE_TEST_OBJS += reftable/record_test.o
 REFTABLE_TEST_OBJS += reftable/readwrite_test.o
 REFTABLE_TEST_OBJS += reftable/stack_test.o
diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
index bae731669c..86a2b0f91a 100644
--- a/t/helper/test-reftable.c
+++ b/t/helper/test-reftable.c
@@ -9,7 +9,6 @@  int cmd__reftable(int argc, const char **argv)
 	record_test_main(argc, argv);
 	block_test_main(argc, argv);
 	tree_test_main(argc, argv);
-	pq_test_main(argc, argv);
 	readwrite_test_main(argc, argv);
 	merged_test_main(argc, argv);
 	stack_test_main(argc, argv);
diff --git a/reftable/pq_test.c b/t/unit-tests/t-reftable-pq.c
similarity index 64%
rename from reftable/pq_test.c
rename to t/unit-tests/t-reftable-pq.c
index b7d3c80cc7..dcde73de66 100644
--- a/reftable/pq_test.c
+++ b/t/unit-tests/t-reftable-pq.c
@@ -6,35 +6,28 @@  license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
-#include "system.h"
-
-#include "basics.h"
-#include "constants.h"
-#include "pq.h"
-#include "record.h"
-#include "reftable-tests.h"
-#include "test_framework.h"
+#include "test-lib.h"
+#include "reftable/constants.h"
+#include "reftable/pq.h"
 
 void merged_iter_pqueue_check(struct merged_iter_pqueue pq)
 {
-	int i;
-	for (i = 1; i < pq.len; i++) {
+	for (int i = 1; i < pq.len; i++) {
 		int parent = (i - 1) / 2;
-
-		EXPECT(pq_less(&pq.heap[parent], &pq.heap[i]));
+		check(pq_less(&pq.heap[parent], &pq.heap[i]));
 	}
 }
 
 static void test_pq(void)
 {
-	struct merged_iter_pqueue pq = { NULL };
+	struct merged_iter_pqueue pq = { 0 };
 	struct reftable_record recs[54];
-	int N = ARRAY_SIZE(recs) - 1, i;
+	size_t N = ARRAY_SIZE(recs) - 1, i;
 	char *last = NULL;
 
 	for (i = 0; i < N; i++) {
 		struct strbuf refname = STRBUF_INIT;
-		strbuf_addf(&refname, "%02d", i);
+		strbuf_addf(&refname, "%02ld", (long)i);
 
 		reftable_record_init(&recs[i], BLOCK_TYPE_REF);
 		recs[i].u.ref.refname = strbuf_detach(&refname, NULL);
@@ -48,7 +41,6 @@  static void test_pq(void)
 
 		merged_iter_pqueue_add(&pq, &e);
 		merged_iter_pqueue_check(pq);
-
 		i = (i * 7) % N;
 	} while (i != 1);
 
@@ -56,9 +48,9 @@  static void test_pq(void)
 		struct pq_entry e = merged_iter_pqueue_remove(&pq);
 		merged_iter_pqueue_check(pq);
 
-		EXPECT(reftable_record_type(e.rec) == BLOCK_TYPE_REF);
+		check(reftable_record_type(e.rec) == BLOCK_TYPE_REF);
 		if (last)
-			EXPECT(strcmp(last, e.rec->u.ref.refname) < 0);
+			check_int(strcmp(last, e.rec->u.ref.refname), <, 0);
 		last = e.rec->u.ref.refname;
 	}
 
@@ -67,8 +59,9 @@  static void test_pq(void)
 	merged_iter_pqueue_release(&pq);
 }
 
-int pq_test_main(int argc, const char *argv[])
+int cmd_main(int argc, const char *argv[])
 {
-	RUN_TEST(test_pq);
-	return 0;
+	TEST(test_pq(), "pq works");
+
+	return test_done();
 }