On Fri, Dec 3, 2021 at 4:19 AM Ævar Arnfjörð Bjarmason [off-list ref] wrote:

On Thu, Dec 02 2021, Eric Sunshine wrote:

quoted

      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.