Labnann <khalid.masum.92@xxxxxxxxx> writes:
> In the test 'cherry-pick after renaming branch', stop checking for
> the presence of a file (opos) because we are going to "grep" in it in
> the same test and the lack of it will be noticed as a failure anyway.
OK.
> In the test 'revert after renaming branch', instead of allowing any
> random contents as long as a known phrase is not there in it, we can
> expect the exact outcome---after the successful revert of
> "added", the contents of file "spoo" should become identical to
> what was in file "oops" in the "initial" commit.
Makes sense. The code removes "test -f spoo", which is not
explained in the above, and I do not think we want to. We'd
notice a breakage where revert leaves the right result in HEAD
but fails to match the working tree files if we leave it there.
> In both tests, we would not notice when "git rev-parse" starts
> segfaulting without emitting any output. The 'test' command would
> end up being just "test =", which yields success. Therefore we could
> use test_cmp_rev
The two sentences are right. The conclusion is a bit iffy; it is
not "we could", which implies that the current one is OK but it is
OK to also rewrite it to use test_cmp_rev.
Use the 'test_cmp_rev' helper to make sure we will notice such a
breakage.
or something like that, perhaps?
> Signed-off-by: Labnann <khalid.masum.92@xxxxxxxxx>
Is Labnann your name? I do not know where you are from so forgive
me if it is. We prefer people to sign with their real names (cf.
Documentation/SubmittingPatches[[real-name]]) on this line (and
because the name/e-mail on this line must match the author's ident,
the same preference applies there, too).
> ---
> t/t3501-revert-cherry-pick.sh | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index 8617efaaf1..ad8f0cae5a 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -66,8 +66,7 @@ test_expect_success 'cherry-pick after renaming branch' '
>
> git checkout rename2 &&
> git cherry-pick added &&
> - test $(git rev-parse HEAD^) = $(git rev-parse rename2) &&
> - test -f opos &&
> + test_cmp_rev rename2 HEAD^ &&
> grep "Add extra line at the end" opos &&
> git reflog -1 | grep cherry-pick
>
> @@ -77,9 +76,8 @@ test_expect_success 'revert after renaming branch' '
>
> git checkout rename1 &&
> git revert added &&
> - test $(git rev-parse HEAD^) = $(git rev-parse rename1) &&
> - test -f spoo &&
> - ! grep "Add extra line at the end" spoo &&
> + test_cmp_rev rename1 HEAD^ &&
> + test_cmp_rev initial:oops HEAD:spoo &&
> git reflog -1 | grep revert
>
> '
The diff looks almost perfect here (modulo that "test -f spoo" is
better kept, I would think).
Thanks.
