Re: [PATCH v7 1/6] t5323: test cases for git-pack-redundant

Junio C Hamano [off-list ref] 于2019年2月1日周五 上午5:44写道：

quoted

+create_commits () {
+     parent=
+     for name in A B C D E F G H I J K L M N O P Q R
+     do
+             test_tick &&
+             T=$(git write-tree) &&

Move this outside loop, not for efficiency but for clarity. This
helper function creates a single empty tree and bunch of commits
that hold the same empty tree, arranged as a single strand of
pearls.

Will rewrite as:

    create_commits () {
            parent=
            T=$(git write-tree) &&
            for name in A B C D E F G H I J K L M N O P Q R

By the way, I had to draw a table like this to figure out ...

     T A B C D E F G H I J K L M N O P Q R
1    x x x x x x x                       x
2        x x x x   x x x
3                x     x x x x x
4                        x x x x     x
5                  x x           x x
6                                x x   x
7                                    x x
8      x

... what is going on.  Perhaps something like this would help other
readers near the top of the file (or in test_description)?

Nice chart, will edit test_description as follows:

    test_description='git pack-redundant test

    In order to test git-pack-redundant, we will create a number of
redundant
    packs in the repository `master.git`. The relationship between
packs (P1-P8)
    and objects (T,A-R) is show in the following chart:

           | T A B C D E F G H I J K L M N O P Q R
        ---+--------------------------------------
        P1 | x x x x x x x                       x
        P2 |     x x x x   x x x
        P3 |             x     x x x x x
        P4 |                     x x x x     x
        P5 |               x x           x x
        P6 |                             x x   x
        P7 |                                 x x
        P8 |   x

    Another repoisitory `shared.git` has unique objects (X-Z), while
share others
    objects through alt-odb (of `master.git`).  The relationship
between packs
    and objects is as follows:

           | T A B C D E F G H I J K L M N O P Q R   X Y Z
        ---+----------------------------------------------
        Px1|   x x x                                 x x x
        Px2|         x x x                           x x x
    '

quoted

+format_packfiles () {
+     sed \
+             -e "s#.*/pack-\(.*\)\.idx#\1#" \
+             -e "s#.*/pack-\(.*\)\.pack#\1#" |
+     sort -u |
+     while read p
+     do
+             if test -z "$(eval echo \${P$p})"
+             then
+                     echo $p

All the "expected output" below will expect P$n:${P$n} prepared by
various create_pack_$n helpers we saw earlier, so an unknown
packfile would be detected as a line that this emits.  Is that the
idea?

Right.  During the reroll, a typo makes an empty output, so I decide
to make this change.

quoted

+             else
+                     eval echo "\${P$p}"
+             fi
+     done |
+     sort
+}
+
+test_expect_success 'setup master.git' '
+     git init --bare master.git &&
+     cd master.git &&
+     create_commits
+'

Everything below will be done inside master.git?  Avoid cd'ing
around in random places in the test script, as a failure in any of
the steps that does cd would start later tests in an unexpected
place, if you can.

The first 10 test cases will run inside master.git, and others will
run inside shared.git.  Only run cd inside the two `setup` test cases.

quoted

+cat >expected <<EOF
+P2:$P2
+EOF
+
+test_expect_success 'one of pack-2/pack-3 is redundant' '
+     git pack-redundant --all >out &&
+     format_packfiles <out >actual &&
+     test_cmp expected actual
+'

Do the preparation of file "expect" (most of the tests compare
'expect' vs 'actual', not 'expected') _inside_ the next test that
uses it.  i.e.

        test_expect_success 'with 1 4 and 5, either 2 or 3 can be omitted' '
                cat >expect <<-EOF &&
                P2:$P2
                EOF
                git pack-redundant --all >out &&
                format ... >actual &&
                test_cmp expect actual
        '

Will do.

quoted

+test_expect_success 'setup shared.git' '
+     cd "$TRASH_DIRECTORY" &&
+     git clone -q --mirror master.git shared.git &&

Why "-q"?

To make verbose output cleaner.

quoted

+     cd shared.git &&
+     printf "../../master.git/objects" >objects/info/alternates
+'

Why not echo?  I recall designing the alternates file to be a plain
text file.  Is it necessary to leave the line incomplete?

Forgot "\n", will append.

quoted

+test_expect_success 'remove redundant packs by alt-odb, no packs left' '
+     git pack-redundant --all --alt-odb | xargs rm &&
+     git fsck --no-progress &&

Why "--no-progress"?

To make verbose output cleaner.

quoted

+     test_must_fail git pack-redundant --all --alt-odb >actual 2>&1 &&
+     test_cmp expected actual
+'
+
+create_commits_others () {
+     parent=$(git rev-parse HEAD)

Will append "&&".