From: Patrick Steinhardt <ps@pks•im>
To: Seyi Kuforiji <kuforiji98@gmail•com>
Cc: git@vger•kernel.org, phillip.wood@dunelm•org.uk
Subject: Re: [PATCH 3/4] t/unit-tests: adapt priority queue test to use clar test framework
Date: Thu, 16 Jan 2025 14:13:02 +0100 [thread overview]
Message-ID: <Z4kF3pqipEZ5WAzH@pks.im> (raw)
In-Reply-To: <20250116104911.77405-4-kuforiji98@gmail.com>
On Thu, Jan 16, 2025 at 11:49:10AM +0100, Seyi Kuforiji wrote:
> diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
> deleted file mode 100644
> index a053635000..0000000000
> --- a/t/unit-tests/t-prio-queue.c
> +++ /dev/null
Hm. A bit surprising that Git decides to not render this as a rename, as
most of `test_prio_queue()` is unchanged.
> diff --git a/t/unit-tests/u-prio-queue.c b/t/unit-tests/u-prio-queue.c
> new file mode 100644
> index 0000000000..d36a565e6f
> --- /dev/null
> +++ b/t/unit-tests/u-prio-queue.c
> @@ -0,0 +1,94 @@
> +#include "unit-test.h"
> +#include "prio-queue.h"
> +
> +static int intcmp(const void *va, const void *vb, void *data UNUSED)
> +{
> + const int *a = va, *b = vb;
> + return *a - *b;
> +}
> +
> +
> +#define MISSING -1
> +#define DUMP -2
> +#define STACK -3
> +#define GET -4
> +#define REVERSE -5
> +
> +static int show(int *v)
> +{
> + return v ? *v : MISSING;
> +}
> +
> +static void test_prio_queue(int *input, size_t input_size,
> + int *result, size_t result_size)
> +{
> + struct prio_queue pq = { intcmp };
> + size_t j = 0;
This is a `size_t` now, which is different compared to before. Might be
worthwhile to point out why you did this in the commit message.
> + for (size_t i = 0; i < input_size; i++) {
> + void *peek, *get;
> + switch(input[i]) {
> + case GET:
> + peek = prio_queue_peek(&pq);
> + get = prio_queue_get(&pq);
> + cl_assert(peek == get);
> + cl_assert(j < result_size);
> + cl_assert_equal_i(result[j], show(get));
> + j++;
> + break;
> + case DUMP:
> + while ((peek = prio_queue_peek(&pq))) {
> + get = prio_queue_get(&pq);
> + cl_assert(peek == get);
> + cl_assert((size_t)j < result_size);
This here is the reason, to avoid -Wsign-compare. But the cast here
isn't necessary now that you've adapted `j` to be a `size_t` anyway.
> + cl_assert_equal_i(result[j], show(get));
> + j++;
> + }
> + break;
> + case STACK:
> + pq.compare = NULL;
> + break;
> + case REVERSE:
> + prio_queue_reverse(&pq);
> + break;
> + default:
> + prio_queue_put(&pq, &input[i]);
> + break;
> + }
> + }
> + cl_assert_equal_i(j, result_size);
> + clear_prio_queue(&pq);
> +}
> +
> +#define TEST_INPUT(input, result) \
> + test_prio_queue(input, ARRAY_SIZE(input), result, ARRAY_SIZE(result))
> +
> +void test_prio_queue__basic(void)
> +{
> + TEST_INPUT(((int []){ 2, 6, 3, 10, 9, 5, 7, 4, 5, 8, 1, DUMP }),
> + ((int []){ 1, 2, 3, 4, 5, 5, 6, 7, 8, 9, 10 }));
> +}
> +
> +void test_prio_queue__mixed(void)
> +{
> + TEST_INPUT(((int []){ 6, 2, 4, GET, 5, 3, GET, GET, 1, DUMP }),
> + ((int []){ 2, 3, 4, 1, 5, 6 }));
> +}
> +
> +void test_prio_queue__empty(void)
> +{
> + TEST_INPUT(((int []){ 1, 2, GET, GET, GET, 1, 2, GET, GET, GET }),
> + ((int []){ 1, 2, MISSING, 1, 2, MISSING }));
> +}
> +
> +void test_prio_queue__stack(void)
> +{
> + TEST_INPUT(((int []){ STACK, 8, 1, 5, 4, 6, 2, 3, DUMP }),
> + ((int []){ 3, 2, 6, 4, 5, 1, 8 }));
> +}
> +
> +void test_prio_queue__reverse_stack(void)
> +{
> + TEST_INPUT(((int []){ STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP }),
> + ((int []){ 1, 2, 3, 4, 5, 6 }));
> +}
All of these look like failthful conversions to me.
Patrick
next prev parent reply other threads:[~2025-01-16 13:13 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-16 10:49 [PATCH 0/4] t/unit-tests: convert unit-tests to use clar Seyi Kuforiji
2025-01-16 10:49 ` [PATCH 1/4] t/unit-tests: handle dashes in test suite filenames Seyi Kuforiji
2025-01-16 13:12 ` Patrick Steinhardt
2025-01-16 10:49 ` [PATCH 2/4] t/unit-tests: convert mem-pool test to use clar test framework Seyi Kuforiji
2025-01-16 13:12 ` Patrick Steinhardt
2025-01-16 17:45 ` Junio C Hamano
2025-01-16 10:49 ` [PATCH 3/4] t/unit-tests: adapt priority queue " Seyi Kuforiji
2025-01-16 13:13 ` Patrick Steinhardt [this message]
2025-01-16 10:49 ` [PATCH 4/4] t/unit-tests: convert reftable tree " Seyi Kuforiji
2025-01-16 16:15 ` [PATCH v2 0/4] t/unit-tests: convert unit-tests to use clar Seyi Kuforiji
2025-01-16 16:15 ` [PATCH v2 1/4] t/unit-tests: handle dashes in test suite filenames Seyi Kuforiji
2025-01-17 6:27 ` Patrick Steinhardt
2025-01-16 16:15 ` [PATCH v2 2/4] t/unit-tests: convert mem-pool test to use clar test framework Seyi Kuforiji
2025-01-16 16:15 ` [PATCH v2 3/4] t/unit-tests: adapt priority queue " Seyi Kuforiji
2025-01-16 16:15 ` [PATCH v2 4/4] t/unit-tests: convert reftable tree " Seyi Kuforiji
2025-01-17 6:27 ` [PATCH v2 0/4] t/unit-tests: convert unit-tests to use clar Patrick Steinhardt
2025-01-17 12:29 ` Seyi Kuforiji
2025-01-17 12:29 ` [PATCH v3 1/4] t/unit-tests: handle dashes in test suite filenames Seyi Kuforiji
2025-01-17 12:29 ` [PATCH v3 2/4] t/unit-tests: convert mem-pool test to use clar test framework Seyi Kuforiji
2025-01-20 10:18 ` Karthik Nayak
2025-01-17 12:29 ` [PATCH v3 3/4] t/unit-tests: adapt priority queue " Seyi Kuforiji
2025-01-20 10:22 ` Karthik Nayak
2025-01-17 12:29 ` [PATCH v3 4/4] t/unit-tests: convert reftable tree " Seyi Kuforiji
2025-01-17 13:36 ` t/unit-tests: convert unit-tests to use clar Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z4kF3pqipEZ5WAzH@pks.im \
--to=ps@pks$(echo .)im \
--cc=git@vger$(echo .)kernel.org \
--cc=kuforiji98@gmail$(echo .)com \
--cc=phillip.wood@dunelm$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox