All of lore.kernel.org
 help / color / mirror / 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:58:25 -0800	[thread overview]
Message-ID: <xmqqa5p3vczi.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")

I forgot to examine the contents of the tests themselves.

> -cat >expect <<'EOF'
> -1
> -2
> -3
> -4
> -5
> -5
> -6
> -7
> -8
> -9
> -10
> -EOF
> -test_expect_success 'basic ordering' '
> -	test-tool prio-queue 2 6 3 10 9 5 7 4 5 8 1 dump >actual &&
> -	test_cmp expect actual
> -'

This seems to have been lost from the converted test.  Your basic
input test feeds an already sorted array of 6 items and dump to see
they are the same already sorted array, which is a lot less
interesting than the above.

> -cat >expect <<'EOF'
> -2
> -3
> -4
> -1
> -5
> -6
> -EOF
> -test_expect_success 'mixed put and get' '
> -	test-tool prio-queue 6 2 4 get 5 3 get get 1 dump >actual &&
> -	test_cmp expect actual
> -'

This is a faithful conversion.

> -cat >expect <<'EOF'
> -1
> -2
> -NULL
> -1
> -2
> -NULL
> -EOF
> -test_expect_success 'notice empty queue' '
> -	test-tool prio-queue 1 2 get get get 1 2 get get get >actual &&
> -	test_cmp expect actual
> -'

This too.

> -cat >expect <<'EOF'
> -3
> -2
> -6
> -4
> -5
> -1
> -8
> -EOF
> -test_expect_success 'stack order' '
> -	test-tool prio-queue stack 8 1 5 4 6 2 3 dump >actual &&
> -	test_cmp expect actual
> -'

This test got truncated in your version, which is not horribly
wrong, but if we claim "move t0009 to unit testing", people would
expect to see a conversion faithful to the original.  And with the
use of result[ARRAY_SIZE(expected)], there is no reason to truncate
the original test with this version, no?

Thanks.

  parent reply	other threads:[~2024-01-17 21:58 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 [this message]
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

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

  git send-email \
    --in-reply-to=xmqqa5p3vczi.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.