Re: [PATCH 1/2] t9902: verify that completion does not print anything

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 3285 bytes --]

On Fri, Jan 12, 2024 at 11:08:21AM +0100, Johannes Schindelin wrote:
> Hi Patrick,
> 
> On Thu, 11 Jan 2024, Patrick Steinhardt wrote:
> 
> > The Bash completion script must not print anything to either stdout or
> > stderr. Instead, it is only expected to populate certain variables.
> > Tighten our `test_completion ()` test helper to verify this requirement.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  t/t9902-completion.sh | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> > index aa9a614de3..78cb93bea7 100755
> > --- a/t/t9902-completion.sh
> > +++ b/t/t9902-completion.sh
> > @@ -87,9 +87,11 @@ test_completion ()
> >  	else
> >  		sed -e 's/Z$//' |sort >expected
> >  	fi &&
> > -	run_completion "$1" &&
> > +	run_completion "$1" >"$TRASH_DIRECTORY"/output 2>&1 &&
> >  	sort out >out_sorted &&
> > -	test_cmp expected out_sorted
> > +	test_cmp expected out_sorted &&
> > +	test_must_be_empty "$TRASH_DIRECTORY"/output &&
> 
> It seems that this fails CI on macOS, most likely because we're running
> with `set -x` and that output somehow ends up in `output`, see e.g. here:
> https://github.com/git/git/actions/runs/7496790359/job/20409405194#step:4:1880
> 
>   [...]
>   ++ test_completion 'git switch '
>   ++ test 1 -gt 1
>   ++ sed -e 's/Z$//'
>   ++ sort
>   ++ run_completion 'git switch '
>   ++ sort out
>   ++ test_cmp expected out_sorted
>   ++ test 2 -ne 2
>   ++ eval 'diff -u' '"$@"'
>   +++ diff -u expected out_sorted
>   ++ test_must_be_empty '/Users/runner/work/git/git/t/trash directory.t9902-completion/output'
>   ++ test 1 -ne 1
>   ++ test_path_is_file '/Users/runner/work/git/git/t/trash directory.t9902-completion/output'
>   ++ test 1 -ne 1
>   ++ test -f '/Users/runner/work/git/git/t/trash directory.t9902-completion/output'
>   ++ test -s '/Users/runner/work/git/git/t/trash directory.t9902-completion/output'
>   ++ echo ''\''/Users/runner/work/git/git/t/trash directory.t9902-completion/output'\'' is not empty, it contains:'
>   '/Users/runner/work/git/git/t/trash directory.t9902-completion/output' is not empty, it contains:
>   ++ cat '/Users/runner/work/git/git/t/trash directory.t9902-completion/output'
>   ++ local -a COMPREPLY _words
>   ++ local _cword
>   [...]
> 
> Maybe this is running in Dash and therefore `BASH_XTRACEFD=4` in
> `test-lib.sh` has not the intended effect?

Meh, thanks for the heads up. Another test gap in GitLab CI which I'm
going to address soon via a new macOS job.

In any case, Dash indeed does not honor the above envvar. I also could
not find any alternate solutions to redirect the tracing output. So for
all I can see there are a few ways to handle this:

  - `set -x` and then restore the previous value after having called
    `run_completion`.

  - Filter the output so that any line starting with "${PS4}" gets
    removed. 

  - Don't test for this bug.

Not sure which way to go, but the first alternative feels a bit more
sensible to me. It does remove the ability to see what's going on in the
completion script though in case one wants to debug it.

I'll reroll this patch series early next week.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]