git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Matheus Tavares <matheus.bernardino@usp.br>
Cc: peff@peff.net, allred.sean@gmail.com, git@vger.kernel.org,
	larsxschneider@gmail.com
Subject: Re: [PATCH] checkout: make delayed checkout respect --quiet and --no-progress
Date: Thu, 26 Aug 2021 01:35:20 +0200	[thread overview]
Message-ID: <87bl5lccx0.fsf@evledraar.gmail.com> (raw)
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..

> +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?

> +
> +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.

> +		(
> +			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?

> +		)
> +	'
> +
> +	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...

  reply	other threads:[~2021-08-25 23:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-21 20:53 Bug report: 'filtering content' delayed progress message does not respect --quiet Sean Allred
2021-03-26  8:31 ` Jeff King
2021-08-25 18:15   ` [PATCH] checkout: make delayed checkout respect --quiet and --no-progress Matheus Tavares
2021-08-25 23:35     ` Ævar Arnfjörð Bjarmason [this message]
2021-08-26 14:26       ` Matheus Tavares Bernardino
2021-08-27  2:26         ` Jeff King
2021-08-26 19:10     ` [PATCH v2] " Matheus Tavares

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bl5lccx0.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=allred.sean@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    --cc=matheus.bernardino@usp.br \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).