SZEDER Gábor <szeder.dev@xxxxxxxxx> 于2019年1月9日周三 下午8:56写道：
>
> On Wed, Jan 02, 2019 at 12:34:54PM +0800, Jiang Xin wrote:
> > From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>
> > +test_description='git redundant test'
>
> s/redundant/pack-redundant/ ?
Yes, will correct it.
> > +
> > +. ./test-lib.sh
> > +
> > +create_commits()
> > +{
> > + set -e
> > + parent=
> > + for name in A B C D E F G H I J K L M
> > + do
> > + test_tick
> > + T=$(git write-tree)
> > + if test -z "$parent"
> > + then
> > + sha1=$(echo $name | git commit-tree $T)
>
> There is a considerable effort going on to switch from SHA-1 to a
> different hash function, so please don't add any new $sha1 variable;
> call it $oid or $commit instead.
>
Will do.
> > +
> > +# Create commits and packs
> > +create_commits
> > +create_redundant_packs
>
> Please perform all setup tasks in a test_expect_success block, so we
> get verbose and trace output about what's going on.
Will do like this:
test_expect_success 'setup' '
create_commits &&
create_redundant_packs
'
> Don't use 'set -e', use an &&-chain instead. To fail the test if a
> command in the for loop were to fail you could do something like this:
>
> for ....
> do
> do-this &&
> do-that ||
> return 1
> done
Will do.
> > +
> > +test_expect_success 'clear loose objects' '
> > + git prune-packed &&
> > + test $(find .git/objects -type f | grep -v pack | wc -l) -eq 0
>
> Use something like
>
> find .git/objects -type f | grep -v pack >out &&
> test_must_be_empty out
>
> instead, so we get an informative error message on failure.
if `grep -v pack` return empty output, it will return error, so
I will use `sed -e "/objects\/pack\//d" >out` instead.
>
> > +'
> > +
> > +cat >expected <<EOF
> > +P1:$P1
> > +P4:$P4
> > +P5:$P5
> > +P6:$P6
> > +EOF
> > +
> > +test_expect_success 'git pack-redundant --all' '
> > + git pack-redundant --all | \
>
> Don't run a git command (especially the particular command the test
> script focuses on) upstream of a pipe, because it hides the command's
> exit code. Use an intermediate file instead.
>
> > + sed -e "s#^.*/pack-\(.*\)\.\(idx\|pack\)#\1#g" | \
>
> This sed command doesn't seem to work on macOS (on Travis CI), and
> causes the test to fail with:
>
It works if rewrite as follows:
git pack-redundant --all >out &&
sed -E -e "s#.*/pack-(.*)\.(idx|pack)#\1#" out | \
Without `-E`, MasOS has to write two seperate sed commands, such as:
git pack-redundant --all >out &&
sed -e "s#.*/pack-\(.*\)\.idx#\1#" out | \
sed -e "s#.*/pack-\(.*\)\.pack#\1#"
Option '-E' is an alias for -r in GNU sed 4.2 (added in 4.2, not documented
unti 4.3), released on May 11 2009. I prefer the `-E` version.
>
> Minor nit: 'git pack-redundant' prints one filename per line, so the
> 'g' at the end of the 's###g' is not necessary.
>
> > + sort -u | \
> > + while read p; do eval echo "\${P$p}"; done | \
> > + sort > actual && \
>
> Style nit: no space between redirection operator and filename
Will do.
