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

On Fri, Feb 1, 2019 at 12:44 AM Jiang Xin [off-list ref] wrote:

quoted

Junio C Hamano [off-list ref] 于2019年2月1日周五 上午5:44写道：
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

Don't forget the && at the end of the 'parent=' line to protect
against someone later adding code above that line. So:

    create_commits () {
        parent= &&
        T=$(git write-tree) &&
        ...

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

test_description should be a meaningful one-liner; it should not
contain this other information, but this information should appear as
comments in the test script.

    Another repoisitory `shared.git` has unique objects (X-Z), while
share others

s/repoisitory/repository/

quoted

quoted

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

That's not what Junio meant. It's okay for tests to 'cd', but each
test which does so _must_ ensure that the 'cd' is undone at the end of
the test, even if the test fails. The correct way to do this within
each test is by using 'cd' in a subhsell, like this:

    test_expect_success 'setup master.git' '
        git init --bare master.git &&
        (
            cd master.git &&
            create_commits
        )
    '

Then, each test which needs to use "master.git" would 'cd' itself, like this:

    test_expect_success 'some test' '
        (
            cd master.git &&
            ...
        )
    '

quoted

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.

What Junio really meant by asking that question was that you should
not do this. When something goes wrong with a test, we want as much
output as possible to help diagnose the problem, so suppressing output
is undesirable. To summarize, don't use -q, --no-progress, or any
other such option and don't redirect to /dev/null.