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

[Alexey Shumkin <alex.crezoff@gmail.com>]

> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> > The expected SHA-1 digests are always available in variables.  Use
> > them instead of hardcoding.
> >
> > Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
> > ---
> >  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.
Ok!

> 
> > +	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 did not spend too much time to think of names of variables at the
moment I was writing this code )
> 
> 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?
It's an inherited code from 1.5 years ago Git ;) taken from some other
tests
I'll change it as you propose )

> 
> > +	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.
yep! you're definitely right

> 
> > +	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.