Re: [PATCH v3] tests: move t0009-prio-queue.sh to the new unit testing framework

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2024, 21:52 UTC
Message-ID: <xmqqh6jbvd9u.fsf@gitster.g>
In-Reply-To: <pull.1642.v3.git.1705502304219.gitgitgadget@gmail.com>

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

Show 9 quoted lines

> +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;
		}
	}

Show 9 quoted lines

> +...
> +			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.

Show 18 quoted lines

> +}
> +
> +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.