Hi Eric,
On Tue, 16 Oct 2018, Eric Sunshine wrote:
> On Tue, Oct 16, 2018 at 7:09 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> > > This cleanup "checkout" needs to be encapsulated within a
> > > test_when_finished(), doesn't it? Preferably just after the "git
> > > checkout -b" invocation.
> >
> > In the meantime, here is what I'll have in 'pu' on top.
> >
> > diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> > @@ -119,12 +119,14 @@ test_expect_success 'git branch `--show-current` works properly when tag exists'
> > cat >expect <<-\EOF &&
> > branch-and-tag-name
> > EOF
> > - test_when_finished "git branch -D branch-and-tag-name" &&
> > + test_when_finished "
> > + git checkout branch-one
> > + git branch -D branch-and-tag-name
> > + " &&
> > git checkout -b branch-and-tag-name &&
> > test_when_finished "git tag -d branch-and-tag-name" &&
> > git tag branch-and-tag-name &&
> > git branch --show-current >actual &&
> > - git checkout branch-one &&
> > test_cmp expect actual
> > '
>
> This make sense to me.
>
> > @@ -137,8 +139,7 @@ test_expect_success 'git branch `--show-current` works properly with worktrees'
> > git worktree add worktree branch-two &&
> > (
> > git branch --show-current &&
> > - cd worktree &&
> > - git branch --show-current
> > + git -C worktree branch --show-current
> > ) >actual &&
> > test_cmp expect actual
> > '
>
> The subshell '(...)' could become '{...}' now that the 'cd' is gone,
> but that's a minor point.
Maybe not so minor.
I realized yesterday that the &&-chain linting we use for every single
test case takes a noticeable chunk of time:
$ time ./t0006-date.sh --quiet
# passed all 67 test(s)
1..67
real 0m20.973s
user 0m2.662s
sys 0m14.789s
$ time ./t0006-date.sh --quiet --no-chain-lint
# passed all 67 test(s)
1..67
real 0m13.607s
user 0m1.330s
sys 0m8.070s
My suspicion: it is essentially the `(exit 117)` that adds about 100ms to
every of those 67 test cases.
(Remember: a subshell requires a fork, and the `fork()` emulation on
Windows requires all kinds of things to be copied to a new process,
including memory and open file descriptors, before the `exec()` will undo
at least part of that.)
With that in mind, I would like to suggest that we should start to be very
careful about using subshells in our test suite.
Ciao,
Dscho
