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

>
> On Fri, Feb 1, 2019 at 12:44 AM Jiang Xin <worldhello.net@gmail.com> wrote:
> >> Junio C Hamano <gitster@pobox.com> 于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.

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

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