On Fri, Dec 3, 2021 at 4:19 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
> On Thu, Dec 02 2021, Eric Sunshine wrote:
> > git worktree add --detach orig &&
> > sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
> > mv orig moved &&
> > - git worktree repair moved >out 2>err &&
> > + git worktree repair moved 2>err &&
> > test_cmp expect .git/worktrees/orig/gitdir &&
> > - test_i18ngrep "gitdir incorrect" out &&
> > - test_must_be_empty err
> > + test_i18ngrep "gitdir incorrect" err
> > '
>
> This is just a "for bonus points", but maybe we could/should while we're
> at it harden and make the tests more exhaustive by checking the full
> output of both, e.g.
>
> cat >actual.out <<-\EOF &&
> Preparing worktree (checking out 'bar')
> EOF
> cat >actual.err <<-\EOF &&
> fatal: 'bar' is already checked out at '.../wherever'
> EOF
> <cmd> [...]
> test_cmp expect.out actual.out &&
> test_cmp expect.err actual.err
>
> Doesn't need a re-roll etc., just if you're interested... :)
To be clear, with the application of the current patch, both of those
messages would need to be in the `actual.err` file, and `actual.out`
would be empty; not one message in each file as in your snippet.
That aside, there's still potentially output which is outside the
control of git-worktree. In the case of this particular negative test,
your suggestion should work, but for a positive test, it would be
harder and uglier (though, of course, not impossible). For instance,
for a successful `git worktree add`, the output is:
Preparing worktree (new branch 'foobar')
Updating files: 100% (3993/3993), done.
HEAD is now at abe6bb3905 Gobbledygook
The subsequent lines come from git-reset (which, by the way, is
sending "HEAD is now at" to stdout, though they probably should be on
stderr, but that's a separate issue).
Anyhow, such a change to the tests should be a separate topic. The
user-facing problem addressed by the current patch series need not be
held up by a behind-the-scenes change to testing.
