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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux