Re: [PATCH v3 02/10] t5526: stop asserting on stderr literally

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 24, 2022, 11:52 UTC
Message-ID: <220224.8635k8a36n.gmgdl@evledraar.gmail.com>
In-Reply-To: <20220224100842.95827-3-chooglen@google.com>

On Thu, Feb 24 2022, Glen Choo wrote:

> +check_sub() {
> +	NEW_HEAD=$1 &&
> +	cat <<-EOF >$pwd/expect.err.sub

Hrm, I didn't know that would work, the usual style is:

    cat >file <<...

Instead of:

    cat <<.. >file

Maybe better to use that?

Show 22 quoted lines

> +	Fetching submodule submodule
> +	From $pwd/submodule
> +	   OLD_HEAD..$NEW_HEAD  sub        -> origin/sub
> +	EOF
> +}
> +
> +check_deep() {
> +	NEW_HEAD=$1 &&
> +	cat <<-EOF >$pwd/expect.err.deep
> +	Fetching submodule submodule/subdir/deepsubmodule
> +	From $pwd/deepsubmodule
> +	   OLD_HEAD..$NEW_HEAD  deep       -> origin/deep
> +	EOF
> +}
> +
> +check_super() {
> +	NEW_HEAD=$1 &&
> +	cat <<-EOF >$pwd/expect.err.super
> +	From $pwd/.
> +	   OLD_HEAD..$NEW_HEAD  super      -> origin/super
> +	EOF
> +}

These look a lot better, but instead of always passing the result of "git rev-parse --short HEAD" can't we just always invoke that in these helpers?

Maybe there are cases where $NEW_HEAD is different, I've just skimmed this series.

Show 8 quoted lines

> @@ -62,7 +82,8 @@ verify_fetch_result() {
>  	if [ -f expect.err.deep ]; then
>  		cat expect.err.deep >>expect.err.combined
>  	fi &&
> -	test_cmp expect.err.combined $ACTUAL_ERR
> +	sed -E 's/[0-9a-f]+\.\./OLD_HEAD\.\./' $ACTUAL_ERR >actual.err.cmp &&
> +	test_cmp expect.err.combined actual.err.cmp
>  }

I think this is unportable per check-non-portable-shell.pl:

        /\bsed\s+-[^efn]\s+/ and err 'sed option not portable (use only -n, -e, -f)';