Subject: Re: [PATCH v2 3/4] test-lib-functions: add helper for trailing hash

From: SZEDER Gábor <[email-masked]>
Date: Sat, 17 Dec 2022 18:37:58 +0100

On Tue, Dec 13, 2022 at 09:55:47AM +0900, Junio C Hamano wrote:
> SZEDER Gábor <[email-masked]> writes:
> 
> >> +	test_trailing_hash .git/index >hash &&
> >> +	echo $(test_oid zero) >expect &&
> >
> > Nit: test_oid zero >expect
> >
> >> +	test_cmp expect hash &&
> 
> Unfortunately they are not equivalent.
> 
> Usually we write these helpers to terminate their output with LF,
> relying on the fact that terminating LF will be dropped when used in
> a command substition, e.g. VAR=$(HELPER), but test_oid deviates from
> the pattern and does not give the terminating LF to its output.

Oh, indeed.  But why does it omit the trailing LF?!  Alas, 2c02b110da
(t: add test functions to translate hash-related values, 2018-09-13)
doesn't seem to mention anything about it.  However, skimming through
the output of

  git grep 'test_oid ' -- ':/t/*.sh'

it appears that all but three uses of 'test_oid' are in command
substitutions, and those three exceptions are in t0000 checking that
'test_oid' actually works.  So I don't see any benefit of omitting
that trailing LF, but this and similar cases show its drawbacks.

  $ git grep 'echo $(test_oid ' -- ':/t/*.sh'
  t/t1302-repo-version.sh:        echo $(test_oid version) >expect &&
  t/t5313-pack-bounds-checks.sh:  echo $(test_oid oidfff) >file &&