Re: [PATCH v2 4/8] test-lib-functions: add and use test_cmp_cmd

[Eric Sunshine <sunshine@sunshineco.com>]

On Thu, Dec 1, 2022 at 8:33 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Thu, Dec 1, 2022 at 7:07 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> > +# test_cmp_cmd is a convenience helper for doing the more verbose:
> > +#
> > +#      echo something >expect &&
> > +#      <some-command-and-args> >actual &&
> > +#      test_cmp expect actual
> > +#
> > +# As:
> > +#
> > +#      test_cmp_cmd something <some-command-and-args>
> > +test_cmp_cmd () {
> > +       local expect="$1" &&
> > +       shift &&
> > +       printf "%s\n" "$expect" >expect &&
> > +       "$@" >actual 2>err &&
> > +       test_must_be_empty err
> > +       test_cmp expect actual
> > +}
>
> I think you also need to document the fact that it expects stderr
> output to be empty.

That said, the behavior of expecting stderr output to be empty seems
like a potentially bad default since it won't play well with commands
which send their "chatty" output to stderr.

Another reason I'm "meh" about this function is that it seems too
narrowly focussed, insisting that the "expect" argument is expressed
as a one-liner. (Yes, I know that that is not a hard limitation; a
caller can pass in a multiline string, but still...) Maybe I'd be less
jaded if it accepted "expect" on its stdin. But, even that doesn't
seem to buy much. The vast majority of cases where you've converted:

    test "$dir" = "$(test-tool path-utils real_path $dir2)" &&

to:

    test_cmp_cmd "$dir" test-tool path-utils real_path $dir2 &&

could just have easily become:

    echo "$dir" >expect &&
    test_cmp test-tool path-utils real_path $dir2 &&

which isn't bad at all, even if it is one line longer, and it is
idiomatic in this test suite.