Re: [PATCH v4 1/4] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Alexey Shumkin <alex.crezoff@xxxxxxxxx> writes:

> The expected SHA-1 digests are always available in variables.  Use
> them instead of hardcoding.
>
> Signed-off-by: Alexey Shumkin <Alex.Crezoff@xxxxxxxxx>
> ---
>  t/t6006-rev-list-format.sh | 130 +++++++++++++++++++++++++--------------------
>  1 file changed, 72 insertions(+), 58 deletions(-)
>
> diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
> index f94f0c4..c248509 100755
> --- a/t/t6006-rev-list-format.sh
> +++ b/t/t6006-rev-list-format.sh
> @@ -6,8 +6,19 @@ test_description='git rev-list --pretty=format test'
>  
>  test_tick
>  test_expect_success 'setup' '
> -touch foo && git add foo && git commit -m "added foo" &&
> -  echo changed >foo && git commit -a -m "changed foo"
> +	touch foo &&

This is inherited from the original, but these days we try to avoid
touch, unless it is about setting a new file timestamp.  The
canonical (in the script we write) way to create an empty file is:

    : >foo

with or without ": ", it does not matter that much.

> +	git add foo &&
> +	git commit -m "added foo" &&
> +	head1=$(git rev-parse --verify HEAD) &&
> +	head1_7=$(echo $head1 | cut -c1-7) &&

Why do we want "whatever_7" variables and use "cut -c1-7" to produce
them?  Is "7" something we care deeply about?

I think what we care a lot more than "7" that happens to be the
current default value is to make sure that, if we ever update the
default abbreviation length to a larger value, the abbreviation
shown with --format=%h is consistent with the abbreviation that is
given by rev-parse --short.

    head1_short=$(git rev-parse --short $head1)

perhaps?

> +	echo changed >foo &&
> +	git commit -a -m "changed foo" &&
> +	head2=$(git rev-parse --verify HEAD) &&
> +	head2_7=$(echo $head2 | cut -c1-7) &&
> +	head2_parent=$(git cat-file -p HEAD | grep parent | cut -f 2 -d" ") &&

Do not use "cat-file -p" that is for human consumption in scripts,
unless you are testing how the format for human consumption should
look like (we may later add more pretty-print to them), which is not
the case here.

Also be careful and pay attention to the end of the header; you do
not want to pick up a random "parent" string in the middle of a log
message.

    head2_parent=$(git cat-file commit HEAD | sed -n -e "s/^parent //p" -e "/^$/q")

would be much better.

> +	head2_parent_7=$(echo $head2_parent | cut -c1-7) &&
> +	tree2=$(git cat-file -p HEAD | grep tree | cut -f 2 -d" ") &&

Likewise.

> +	tree2_7=$(echo $tree2 | cut -c1-7)

Likewise.

> @@ -131,39 +142,42 @@ This commit message is much longer than the others,
>  and it will be encoded in iso8859-1. We should therefore
>  include an iso8859 character: ¡bueno!
>  EOF
> +
>  test_expect_success 'setup complex body' '
> -git config i18n.commitencoding iso8859-1 &&
> -  echo change2 >foo && git commit -a -F commit-msg
> +	git config i18n.commitencoding iso8859-1 &&
> +	echo change2 >foo && git commit -a -F commit-msg &&
> +	head3=$(git rev-parse --verify HEAD) &&
> +	head3_7=$(echo $head3 | cut -c1-7)
>  '

Likewise.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]