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>,
	Jeff King <peff@peff.net>,
	 Phillip Wood <phillip.wood123@gmail.com>,
	Chandra Pratap <chandrapratap376@gmail.com>
Subject: Re: [PATCH v4] tests: move t0009-prio-queue.sh to the new unit testing framework
Date: Mon, 22 Jan 2024 11:09:39 -0800	[thread overview]
Message-ID: <xmqqwms1tcb0.fsf@gitster.g> (raw)
In-Reply-To: <pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com> (Chandra Pratap via GitGitGadget's message of "Sun, 21 Jan 2024 19:28:45 +0000")

> diff --git a/t/unit-tests/t-prio-queue.c b/t/unit-tests/t-prio-queue.c
> new file mode 100644
> index 00000000000..d78b002f9ea
> --- /dev/null
> +++ b/t/unit-tests/t-prio-queue.c
> @@ -0,0 +1,98 @@
> +#include "test-lib.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, int *result, size_t input_size)
> +{
> +	struct prio_queue pq = { intcmp };
> +
> +	for (int i = 0, j = 0; i < input_size; i++) {
> +		void *peek, *get;
> +		switch(input[i]) {

Style: in this codebase, a flow control keyword followed by a
parenthesized stuff always get a single SP before the parenthesis.

		switch (input[i]) {

There are similar style violations in this patch with if().

> +		case GET:
> +			peek = prio_queue_peek(&pq);
> +			get = prio_queue_get(&pq);
> +			if (!check(peek == get))
> +				return;
> +			if(!check_int(result[j++], ==, show(get)))
> +				test_msg("failed at result[] index %d", j-1);
> +			break;
> +		case DUMP:
> +			while ((peek = prio_queue_peek(&pq))) {
> +				get = prio_queue_get(&pq);
> +				if (!check(peek == get))
> +					return;
> +				if(!check_int(result[j++], ==, show(get)))
> +					test_msg("failed at result[] index %d", j-1);
> +			}
> +			break;

OK.  So this one checks as we go.  I am not sure how easy to grok a
breakage diagnosis with these giving the same message, without
giving any context of the failure (e.g. when we are fed

	INPUT  = 6 2 4 GET 5 3 GET GET 1 DUMP
	EXPECT = 2 3 4 1 5 6

and for some reason if the first GET did not yield expected 2 but
gave us, say, 6, we only see "left: 2, right: 6" followed by "failed
at result[] index 0", and nothing else.  

If it said something like "We pushed 6, 2, 4 and then did GET" to
give the reader a bit more context, it would make it easier to see
why we were complaining, i.e. expecting to see 2, instead getting 6.
But perhaps that is too much to ask to this code?

I dunno.  Those who wanted to see an easier-to-diagnose output may
have better ideas.

Thanks.

> +		case STACK:
> +			pq.compare = NULL;
> +			break;
> +		case REVERSE:
> +			prio_queue_reverse(&pq);
> +			break;
> +		default:
> +			prio_queue_put(&pq, &input[i]);
> +			break;
> +		}
> +	}
> +	clear_prio_queue(&pq);
> +}
> +
> +#define BASIC_INPUT 2, 6, 3, 10, 9, 5, 7, 4, 5, 8, 1, DUMP
> +#define BASIC_RESULT 1, 2, 3, 4, 5, 5, 6, 7, 8, 9, 10
> +
> +#define MIXED_PUT_GET_INPUT 6, 2, 4, GET, 5, 3, GET, GET, 1, DUMP
> +#define MIXED_PUT_GET_RESULT 2, 3, 4, 1, 5, 6
> +
> +#define EMPTY_QUEUE_INPUT 1, 2, GET, GET, GET, 1, 2, GET, GET, GET
> +#define EMPTY_QUEUE_RESULT 1, 2, MISSING, 1, 2, MISSING
> +
> +#define STACK_INPUT STACK, 8, 1, 5, 4, 6, 2, 3, DUMP
> +#define STACK_RESULT 3, 2, 6, 4, 5, 1, 8
> +
> +#define REVERSE_STACK_INPUT STACK, 1, 2, 3, 4, 5, 6, REVERSE, DUMP
> +#define REVERSE_STACK_RESULT 1, 2, 3, 4, 5, 6
> +
> +#define TEST_INPUT(INPUT, RESULT, name)			\
> +  static void test_##name(void)				\
> +{								\
> +	int input[] = {INPUT};					\
> +	int result[] = {RESULT};				\
> +	test_prio_queue(input, result, ARRAY_SIZE(input));	\
> +}
> +
> +TEST_INPUT(BASIC_INPUT, BASIC_RESULT, basic)
> +TEST_INPUT(MIXED_PUT_GET_INPUT, MIXED_PUT_GET_RESULT, mixed)
> +TEST_INPUT(EMPTY_QUEUE_INPUT, EMPTY_QUEUE_RESULT, empty)
> +TEST_INPUT(STACK_INPUT, STACK_RESULT, stack)
> +TEST_INPUT(REVERSE_STACK_INPUT, REVERSE_STACK_RESULT, 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();
> +}
>
> base-commit: 1a87c842ece327d03d08096395969aca5e0a6996


  reply	other threads:[~2024-01-22 19:17 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
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 [this message]
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=xmqqwms1tcb0.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chandrapratap3519@gmail.com \
    --cc=chandrapratap376@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood123@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).