Re: [PATCH 07/15] t0020: drop redirections to /dev/null

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Dec 17, 2019 at 7:02 AM Denton Liu <liu.denton@gmail.com> wrote:
> Since output is silenced when running without `-v` and debugging output
> is useful with `-v`, remove redirections to /dev/null as it is not
> useful.
>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> @@ -5,7 +5,7 @@ test_description='CRLF conversion'
>  has_cr() {
> -       tr '\015' Q <"$1" | grep Q >/dev/null
> +       tr '\015' Q <"$1" | grep Q
>  }

I'm not sure that I agree with this change since dropping >/dev/null
doesn't improve the situation when someone is trying to debug a
failing test. What this change will do is fill the -v output with a
bunch of lines ending with "Q" when CR is expected -- the normal
_successful_ case for about half the calls to has_cr() -- so -v output
will become a lot noisier without really adding much, if any,
debugging value. If you really want to help people trying to diagnose
failures, I could see you replacing has_cr() with two new functions
which actually provide useful diagnostic output; for instance
something like:

    expect_cr () {
        if ! tr '\015' Q <"$1" | grep Q >/dev/null
        then
            echo "missing CR termination in: $1" >&2 &&
            return 1
        fi
    }

    expect_no_cr () {
        if tr '\015' Q <"$1" | grep Q >/dev/null
        then
            echo "unexpected CR termination in: $1" >&2 &&
            return 1
        fi
    }

However, I'm not convinced that introducing and debugging these
functions is worth the effort over simply dropping this patch from the
series.