git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,
	 Chandra Pratap <chandrapratap3519@gmail.com>,
	Chandra Pratap <chandrapratap376@gmail.com>
Subject: Re: [PATCH v3] tests: move t0009-prio-queue.sh to the new unit testing framework
Date: Wed, 17 Jan 2024 13:52:13 -0800	[thread overview]
Message-ID: <xmqqh6jbvd9u.fsf@gitster.g> (raw)
In-Reply-To: <pull.1642.v3.git.1705502304219.gitgitgadget@gmail.com> (Chandra Pratap via GitGitGadget's message of "Wed, 17 Jan 2024 14:38:23 +0000")

"Chandra Pratap via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static int test_prio_queue(int *input, int *result)
> +{
> +	struct prio_queue pq = { intcmp };
> +	int i = 0;
> +
> +	while (*input) {
> +		int *val = input++;
> +		void *peek, *get;
> +		switch(*val) {

Ah, this one is a bit tricky.  I would have expected that we can
make val a pure integer, but because we need to give the address of
the element to be placed in the queue, not the value to be placed as
an element in the queue, to prio_queue_put(), we would need the
pointer.

I wonder if writing this as a more conventional for(;;) loop would
make it easier to handle, i.e.

	int val, i;

	for (i = 0; (val = *input); input++) {
		switch (val) {
		case ...:
		...
		default:
			prio_queue_put(&pq, input);
			break;
		}
	}

> +...
> +			default:
> +				prio_queue_put(&pq, val);
> +				break;
> +		}
> +	}
> +	clear_prio_queue(&pq);
> +	return 0;
> +}

> +#define TEST_INPUT(INPUT, EXPECTED, name)			\
> +  static void test_##name(void)				\
> +{								\
> +	int input[] = {INPUT};					\

I think I missed this myself in my review the last round, but we
need the sentinel value 0 at the end, i.e.,

	int input[] = {INPUT, 0};

because the test_prio_queue() loops "while (*input)".  Otherwise
the loop would not terminate.

> +	int expected[] = {EXPECTED};				\
> +	int result[ARRAY_SIZE(expected)];			\

These arrays are correct.

> +	test_prio_queue(input, result);				\
> +	check(!memcmp(expected, result, sizeof(expected)));	\

So is this check.

> +}
> +
> +TEST_INPUT(BASIC_INPUT, BASIC_EXPECTED, basic)
> +TEST_INPUT(MIXED_PUT_GET_INPUT, MIXED_PUT_GET_EXPECTED, mixed)
> +TEST_INPUT(EMPTY_QUEUE_INPUT, EMPTY_QUEUE_EXPECTED, empty)
> +TEST_INPUT(STACK_INPUT, STACK_EXPECTED, stack)
> +TEST_INPUT(REVERSE_STACK_INPUT, REVERSE_STACK_EXPECTED, reverse)
> +
> +int cmd_main(int argc, const char **argv)
> +{
> +	TEST(test_basic(), "prio-queue works for basic input");
> +	TEST(test_mixed(), "prio-queue works for mixed put & get commands");
> +	TEST(test_empty(), "prio-queue works when queue is empty");
> +	TEST(test_stack(), "prio-queue works when used as a LIFO stack");
> +	TEST(test_reverse(), "prio-queue works when LIFO stack is reversed");
> +
> +	return test_done();
> +}

Other than that, looking good.  Thanks.


  reply	other threads:[~2024-01-17 21:52 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-14  8:10 [PATCH] tests: move t0009-prio-queue.sh to the new unit testing framework Chandra Pratap via GitGitGadget
2024-01-14  8:15 ` Chandra Pratap
2024-01-14  8:18 ` [PATCH v2] " Chandra Pratap via GitGitGadget
2024-01-16 16:36   ` Junio C Hamano
2024-01-17  6:01   ` Josh Steadmon
2024-01-17 14:38   ` [PATCH v3] " Chandra Pratap via GitGitGadget
2024-01-17 21:52     ` Junio C Hamano [this message]
2024-01-17 21:58     ` Junio C Hamano
2024-01-17 22:15       ` Junio C Hamano
2024-01-20  2:31     ` Jeff King
2024-01-20 14:23     ` Phillip Wood
2024-01-21 19:28     ` [PATCH v4] " Chandra Pratap via GitGitGadget
2024-01-22 19:09       ` Junio C Hamano
2024-01-22 22:42         ` Jeff King
2024-01-25 20:02       ` [PATCH v5] " Chandra Pratap via GitGitGadget

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqh6jbvd9u.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chandrapratap3519@gmail.com \
    --cc=chandrapratap376@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).