Re: [PATCH v4] add-patch: response to unknown command

[Eric Sunshine <sunshine@sunshineco.com>]

On Wed, Apr 24, 2024 at 9:44 PM Jeff King <peff@peff.net> wrote:
> On Sun, Apr 21, 2024 at 11:52:33PM +0200, Rubén Justo wrote:
> > +test_expect_success 'unknown command' '
> > +     test_when_finished "git reset --hard; rm -f command" &&
> > +     echo W >command &&
> > +     git add -N command &&
> > +     git diff command >expect &&
> > +     cat >>expect <<-EOF &&
> > +     (1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
> > +     (1/1) Stage addition [y,n,q,a,d,e,p,?]?$SP
> > +     EOF
> > +     git add -p -- command <command >actual 2>&1 &&
> > +     test_cmp expect actual
> > +'
>
> I got a test failure on Windows CI from this. The test_cmp output looks
> like this:
>
>   -(1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command 'W' (use '?' for help)
>   -(1/1) Stage addition [y,n,q,a,d,e,p,?]?
>   +(1/1) Stage addition [y,n,q,a,d,e,p,?]? (1/1) Stage addition [y,n,q,a,d,e,p,?]?
>   +Unknown command 'W' (use '?' for help)
>
> which makes me suspect a race. Perhaps because the prompt is going to
> stdout, but the "Unknown command" message goes to stderr? Maybe we
> should keep stdout and stderr separate and check them independently.

That's very reminiscent of [1]. Although, unlike [1], the output
presented to the user in this case is (I suppose) less likely to be
messed up; only the combined captured output is probably affected. So,
capturing stdout and stderr separately would indeed be a good idea.

[1]: https://lore.kernel.org/git/CAPig+cTGq-10ZTBts2LXRVdPMf2vNMX8HTuhg_+ZHSiLX-brOQ@mail.gmail.com/