From: Eric Sunshine <sunshine@sunshineco.com>
To: Stephan Beyer <s-beyer@gmx.net>
Cc: Git List <git@vger.kernel.org>,
Christian Couder <christian.couder@gmail.com>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 04/21] t: use test_cmp_rev() where appropriate
Date: Sun, 10 Apr 2016 20:07:21 -0400 [thread overview]
Message-ID: <CAPig+cS+4G22ARfwxDnfPPS6A+oFHYtOD-99vh9EOt=qwLoZSg@mail.gmail.com> (raw)
In-Reply-To: <1460294354-7031-5-git-send-email-s-beyer@gmx.net>
On Sun, Apr 10, 2016 at 9:18 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> test_cmp_rev() from t/test-lib-functions.sh is used to make many
> tests clearer.
>
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---
> diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
> @@ -537,8 +537,8 @@ EOF
> test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
> test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
> # Refs are unchanged
> - test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
> - test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
> + test_cmp_rev refs/notes/m refs/notes/w &&
> + test_cmp_rev refs/notes/y NOTES_MERGE_PARTIAL^1 &&
> test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
Does this one deserve the same type of treatment (but prefixed with !)?
! test_cmp_rev refs/notes/m NOTES_MERGE_PARTIAL^1
I could understand avoiding this conversion if temp_cmp_rev was still
noisy upon failure, however patch 3/21 silenced it.
Aside: Would make sense for test_cmp_rev to accept an optional ! as
its first argument to signify that the revs should not match?
> # Mention refs/notes/m, and its current and expected value in output
> grep -q "refs/notes/m" output &&
> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
> @@ -354,8 +354,8 @@ test_expect_success '--remap-to-ancestor with filename filters' '
> - test $(git rev-parse moved-foo) = $(git rev-parse moved-bar) &&
> - test $(git rev-parse moved-foo) = $(git rev-parse master^) &&
> + test_cmp_rev moved-foo moved-bar &&
> + test_cmp_rev moved-foo master^ &&
> test $orig_invariant = $(git rev-parse invariant)
At other places in this patch, you also converted lines such as the
above, so why not here?
test_cmp_rev invariant $orig_invariant
> '
>
> @@ -372,8 +372,8 @@ test_expect_success 'automatic remapping to ancestor with filename filters' '
> - test $(git rev-parse moved-foo2) = $(git rev-parse moved-bar2) &&
> - test $(git rev-parse moved-foo2) = $(git rev-parse master^) &&
> + test_cmp_rev moved-foo2 moved-bar2 &&
> + test_cmp_rev moved-foo2 master^ &&
> test $orig_invariant = $(git rev-parse invariant2)
Ditto.
> '
>
> diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> @@ -79,17 +79,16 @@ test_expect_success 'merge c1 with c2 and c3 (recursive in pull.octopus)' '
> test_expect_success 'merge c1 with c2 and c3 (recursive and octopus in pull.octopus)' '
> git reset --hard c1 &&
> git config pull.octopus "recursive octopus" &&
> git merge c2 c3 &&
> - test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
> - test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
> - test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> - test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
> + test_cmp_rev c1 HEAD^1 &&
> + test_cmp_rev c2 HEAD^2 &&
> + test_cmp_rev c3 HEAD^3 &&
This drops the check:
! test_cmp_rev c1 HEAD &&
Is that intentional? I suppose the argument is that if c1 == HEAD^1
passes, then c1 is certainly not == HEAD?
> diff --git a/t/t7603-merge-reduce-heads.sh b/t/t7603-merge-reduce-heads.sh
> @@ -46,11 +46,10 @@ test_expect_success 'setup' '
> test_expect_success 'merge c1 with c2, c3, c4, c5' '
> git reset --hard c1 &&
> git merge c2 c3 c4 c5 &&
> - test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
> - test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
> - test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> - test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
> - test "$(git rev-parse c5)" = "$(git rev-parse HEAD^4)" &&
> + test_cmp_rev c1 HEAD^1 &&
> + test_cmp_rev c2 HEAD^2 &&
> + test_cmp_rev c3 HEAD^3 &&
> + test_cmp_rev c5 HEAD^4 &&
Ditto.
> @@ -69,11 +68,10 @@ test_expect_success 'merge c1 with c2, c3, c4, c5' '
> test_expect_success 'pull c2, c3, c4, c5 into c1' '
> git reset --hard c1 &&
> git pull . c2 c3 c4 c5 &&
> - test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
> - test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
> - test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> - test "$(git rev-parse c3)" = "$(git rev-parse HEAD^3)" &&
> - test "$(git rev-parse c5)" = "$(git rev-parse HEAD^4)" &&
> + test_cmp_rev c1 HEAD^1 &&
> + test_cmp_rev c2 HEAD^2 &&
> + test_cmp_rev c3 HEAD^3 &&
> + test_cmp_rev c5 HEAD^4 &&
Ditto.
> diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh
> @@ -30,9 +30,8 @@ test_expect_success 'setup' '
> test_expect_success 'merge c1 to c2' '
> git reset --hard c1 &&
> git merge -s resolve c2 &&
> - test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
> - test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
> - test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
> + test_cmp_rev c1 HEAD^1 &&
> + test_cmp_rev c2 HEAD^2 &&
Ditto.
next prev parent reply other threads:[~2016-04-11 0:07 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-10 13:18 [PATCH v2 00/21] git bisect improvements Stephan Beyer
2016-04-10 13:18 ` [PATCH v2 01/21] bisect: write about `bisect next` in documentation Stephan Beyer
2016-04-10 13:18 ` [PATCH v2 02/21] bisect: allow 'bisect run' if no good commit is known Stephan Beyer
2016-04-10 13:18 ` [PATCH v2 03/21] t/test-lib-functions.sh: generalize test_cmp_rev Stephan Beyer
2016-04-11 0:07 ` Eric Sunshine
2016-04-15 20:00 ` Junio C Hamano
2016-04-24 19:51 ` Stephan Beyer
2016-04-25 18:08 ` Junio C Hamano
2016-04-10 13:18 ` [PATCH v2 04/21] t: use test_cmp_rev() where appropriate Stephan Beyer
2016-04-11 0:07 ` Eric Sunshine [this message]
2016-04-15 20:48 ` Junio C Hamano
2016-04-10 13:18 ` [PATCH v2 05/21] t6030: generalize test to not rely on current implementation Stephan Beyer
2016-04-10 13:47 ` Torsten Bögershausen
2016-04-10 19:16 ` Junio C Hamano
2016-04-10 19:37 ` Stephan Beyer
2016-04-11 0:23 ` Eric Sunshine
2016-04-15 21:07 ` Junio C Hamano
2016-04-10 13:18 ` [PATCH v2 06/21] bisect: add test for the bisect algorithm Stephan Beyer
2016-04-15 21:13 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 07/21] bisect: plug the biggest memory leak Stephan Beyer
2016-04-15 21:18 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 08/21] bisect: make bisect compile if DEBUG_BISECT is set Stephan Beyer
2016-04-15 21:22 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 09/21] bisect: make algorithm behavior independent of DEBUG_BISECT Stephan Beyer
2016-04-15 21:25 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 10/21] bisect: get rid of recursion in count_distance() Stephan Beyer
2016-04-15 21:31 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 11/21] bisect: use struct node_data array instead of int array Stephan Beyer
2016-04-12 23:02 ` Christian Couder
2016-04-15 21:47 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 12/21] bisect: replace clear_distance() by unique markers Stephan Beyer
2016-04-12 23:20 ` Christian Couder
2016-04-15 22:07 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 13/21] bisect: use commit instead of commit list as arguments when appropriate Stephan Beyer
2016-04-15 22:08 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 14/21] bisect: extract get_distance() function from code duplication Stephan Beyer
2016-04-15 22:08 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 15/21] bisect: introduce distance_direction() Stephan Beyer
2016-04-15 22:10 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 16/21] bisect: make total number of commits global Stephan Beyer
2016-04-13 13:23 ` Christian Couder
2016-04-15 22:11 ` Junio C Hamano
2016-04-16 0:44 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 17/21] bisect: rename count_distance() to compute_weight() Stephan Beyer
2016-04-13 13:32 ` Christian Couder
2016-04-15 22:12 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 18/21] bisect: prepare for different algorithms based on find_all Stephan Beyer
2016-04-15 22:36 ` Junio C Hamano
2016-04-10 13:19 ` [PATCH v2 19/21] bisect: use a bottom-up traversal to find relevant weights Stephan Beyer
2016-04-13 14:11 ` Christian Couder
2016-04-15 22:47 ` Junio C Hamano
2016-04-15 22:49 ` Junio C Hamano
2016-04-26 18:27 ` Junio C Hamano
2016-04-10 13:24 ` [PATCH v2 20/21] bisect: compute best bisection in compute_relevant_weights() Stephan Beyer
2016-04-10 13:24 ` [PATCH v2 21/21] bisect: get back halfway shortcut Stephan Beyer
2016-04-15 22:53 ` Junio C Hamano
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='CAPig+cS+4G22ARfwxDnfPPS6A+oFHYtOD-99vh9EOt=qwLoZSg@mail.gmail.com' \
--to=sunshine@sunshineco.com \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=s-beyer@gmx.net \
/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).