Re: [PATCH v3 1/3] t5323: test cases for git-pack-redundant

SZEDER Gábor [off-list ref] 于2019年1月9日周三 下午8:56写道：

On Wed, Jan 02, 2019 at 12:34:54PM +0800, Jiang Xin wrote:

quoted

From: Jiang Xin <redacted>
+test_description='git redundant test'

s/redundant/pack-redundant/ ?

Yes, will correct it.

quoted

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

quoted

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

quoted

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

quoted

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

quoted

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

quoted

+             sort -u | \
+             while read p; do eval echo "\${P$p}"; done | \
+             sort > actual && \

Style nit: no space between redirection operator and filename

Will do.