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@xxxxxx>
> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
> Signed-off-by: Chandra Pratap <chandrapratap3519@xxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: PGP signature
