Re: [PATCH v2 1/3] Add tests for describe with --work-tree

From: Sebastian Staudt <koraktor@gmail.com>
Date: Jan 27, 2019, 07:13 UTC
Message-ID: <CA+xP2SbLwmeWpqmjCiqojra5Mwrbok7sjvUsvCsaAo6XsWBbtA@mail.gmail.com>
In-Reply-To: <CACsJy8A6hF+Sxitpt3J9EmDEXjGb2e+AeMtJDaE7Y32fg4NEKA@mail.gmail.com>

Am So., 27. Jan. 2019 um 01:07 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:

Show 23 quoted lines

>
> On Sun, Jan 27, 2019 at 3:51 AM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > The dirty ones are already passing, but just because describe is comparing
> > with the wrong working tree.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> >  t/t6120-describe.sh | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
> > index d639d94696..9a6bd1541f 100755
> > --- a/t/t6120-describe.sh
> > +++ b/t/t6120-describe.sh
> > @@ -28,6 +28,24 @@ check_describe () {
> >         '
> >  }
> >
> > +check_describe_worktree () {
> > +  cd "$TEST_DIRECTORY"
>
> Strange alignment. We normally do it in a subshell...

Sure, will fix this.

Show 22 quoted lines

>
> > +       expect="$1"
> > +       shift
> > +       R=$(git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe "$@" 2>err.actual)
>
> These commands should be executed inside test_expect_success, not
> outside. And you need to chain commands with && to make sure if
> something breaks, then the whole test will fai.
>
> If it's too ugly to generate test_expect_success with a shell
> function, then just write a shell function that "describe" and compare
> (i.e. the test body). Then you can write something like this later
>
> test_expect_sucesss 'describe with --worktree foo' '
>         check_describe_worktree foo
> '
>
> and check_describe_worktree can now do
>
> ( cd "$TEST_DIRECTORY" && .... )
>
>

My function is a modified version of check_describe(). Which does the same thing. I‘m not really experienced in Shell programming, so I didn‘t see a cleaner way. But having the cd commands in the && chain looks broken as it would break the following tests when one test fails and the code was executed in the wrong directory afterwards.

Show 45 quoted lines

>
> > +       S=$?
> > +       cat err.actual >&3
> > +       test_expect_success "describe with --work-tree $*" '
> > +       test $S = 0 &&
> > +       case "$R" in
> > +       $expect)        echo happy ;;
> > +       *)      echo "Oops - $R is not $expect";
> > +               false ;;
> > +       esac
> > +       '
> > +  cd "$TRASH_DIRECTORY"
> > +}
> > +
> >  test_expect_success setup '
> >
> >         test_tick &&
> > @@ -145,14 +163,20 @@ check_describe A-* HEAD
> >
> >  check_describe "A-*[0-9a-f]" --dirty
> >
> > +check_describe_worktree "A-*[0-9a-f]" --dirty
> > +
> >  test_expect_success 'set-up dirty work tree' '
> >         echo >>file
> >  '
> >
> >  check_describe "A-*[0-9a-f]-dirty" --dirty
> >
> > +check_describe_worktree "A-*[0-9a-f]-dirty" --dirty
> > +
> >  check_describe "A-*[0-9a-f].mod" --dirty=.mod
> >
> > +check_describe_worktree "A-*[0-9a-f].mod" --dirty=.mod
> > +
> >  test_expect_success 'describe --dirty HEAD' '
> >         test_must_fail git describe --dirty HEAD
> >  '
> > --
> > 2.20.1
> >
>
>
> --
> Duy