Re: [PATCH] checkout: make delayed checkout respect --quiet and --no-progress

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Aug 25, 2021, 23:35 UTC
Message-ID: <87bl5lccx0.fsf@evledraar.gmail.com>
In-Reply-To: <d1405b781915c085ac8a8965dadf3efbe1b0f6aa.1629915330.git.matheus.bernardino@usp.br>

On Wed, Aug 25 2021, Matheus Tavares wrote:

> The test section of the patch is a bit long because it checks all the
> verbosity options related to the progress report, and it also tests both
> the check_updates() and checkout_worktree() code paths. If that is
> overkill, I can remove some tests.

More exhaustive tests are generally nice..

Show 13 quoted lines

> +test_expect_success PERL 'setup for progress tests' '
> +	git init progress &&
> +	(
> +		cd progress &&
> +		git config filter.delay.process "rot13-filter.pl delay-progress.log clean smudge delay" &&
> +		git config filter.delay.required true &&
> +
> +		echo "*.a filter=delay" >.gitattributes &&
> +		touch test-delay10.a &&
> +		git add . &&
> +		git commit -m files
> +	)
> +'

This doesn't seem to depend on PERL, should this really be a skip_all at the top if we don't have the TTY prereq, i.e. we shouldn't bother?

Show 9 quoted lines

> +
> +for mode in pathspec branch
> +do
> +	case "$mode" in
> +	pathspec) opt='.' ;;
> +	branch) opt='-f HEAD' ;;
> +	esac
> +
> +	test_expect_success PERL,TTY "delayed checkout shows progress by default only on tty ($mode checkout)" '

All of the PERL,TTY can just be TTY, since TTY itself checks PERL.

Show 6 quoted lines

> +		(
> +			cd progress &&
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			grep "Filtering content" err &&

This seems to need TTY...

> +			rm -f *.a delay-progress.log &&
> +			GIT_PROGRESS_DELAY=0 git checkout $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			! grep "Filtering content" err

But this one doesn't, perhaps it could be a non-TTY test?

Show 27 quoted lines

> +		)
> +	'
> +
> +	test_expect_success PERL,TTY "delayed checkout ommits progress with --quiet ($mode checkout)" '
> +		(
> +			cd progress &&
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			! grep "Filtering content" err
> +		)
> +	'
> +
> +	test_expect_success PERL,TTY "delayed checkout honors --[no]-progress ($mode checkout)" '
> +		(
> +			cd progress &&
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --no-progress $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			! grep "Filtering content" err &&
> +
> +			rm -f *.a delay-progress.log &&
> +			test_terminal env GIT_PROGRESS_DELAY=0 git checkout --quiet --progress $opt 2>err &&
> +			grep "IN: smudge test-delay10.a .* \\[DELAYED\\]" delay-progress.log &&
> +			grep "Filtering content" err
> +		)
> +	'

It looks like these tests could be split into one helper function which just passed params for e.g. whether the "Filtering content" grep was negated, and what command should be run.

Also if possible the two sections of the test could be split up, and then the "rm -rf" could just be a "test_when_finished" at the top...