git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Tom Hughes <tom@compton.nu>
Cc: git@vger.kernel.org,  chriscool@tuxfamily.org,  jonathantanmy@google.com
Subject: Re: [PATCH v2] promisor-remote: add promisor.quiet configuration option
Date: Fri, 24 May 2024 11:06:57 -0700	[thread overview]
Message-ID: <xmqqa5kfkrsu.fsf@gitster.g> (raw)
In-Reply-To: <20240524090937.2448229-1-tom@compton.nu> (Tom Hughes's message of "Fri, 24 May 2024 10:09:37 +0100")

Tom Hughes <tom@compton.nu> writes:

> +test_expect_success TTY 'promisor.quiet=false shows progress messages' '
> +	rm -rf server repo &&
> +	test_create_repo server &&
> +	test_commit -C server foo &&
> +	git -C server rm foo.t &&
> +	git -C server commit -m remove &&
> +	git -C server config uploadpack.allowanysha1inwant 1 &&
> +	git -C server config uploadpack.allowfilter 1 &&
> +
> +	git clone --filter=blob:none "file://$(pwd)/server" repo &&
> +	git -C repo config promisor.quiet "false" &&
> +
> +	test_terminal git -C repo cat-file -p foo:foo.t 2>err &&
> +
> +	# Ensure that progress messages are written
> +	grep "Receiving objects" err
> +'
> +
> +test_expect_success TTY 'promisor.quiet=true does not show progress messages' '
> +	rm -rf server repo &&
> +	test_create_repo server &&
> +	test_commit -C server foo &&
> +	git -C server rm foo.t &&
> +	git -C server commit -m remove &&
> +	git -C server config uploadpack.allowanysha1inwant 1 &&
> +	git -C server config uploadpack.allowfilter 1 &&
> +
> +	git clone --filter=blob:none "file://$(pwd)/server" repo &&
> +	git -C repo config promisor.quiet "true" &&
> +
> +	test_terminal git -C repo cat-file -p foo:foo.t 2>err &&
> +
> +	# Ensure that no progress messages are written
> +	! grep "Receiving objects" err
> +'

Wouldn't we want to see this as three (or four) tests,

 (1) "setup" that prepares the server end

 (2) "quiet=false" test that
     - makes a partial clone, 
     - sets .quiet to false, 
     - runs cat-file -p,
     - makes sure that the lazy fetching is chatty.

 (3) "quiet=true" test, which is the same as (2) except that it sets
     .quiet to true and expects the lazy fetching to be silent.

 (4) "quiet=unconfigured" test, which is the same as (2) except that
     it leaves .quiet unconfigured.

Other than that, looking very much nicer than the previous round
that manually mucked with internal implementation on the client end.

Thanks.


  reply	other threads:[~2024-05-24 18:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-23 13:19 [PATCH] promisor-remote: add promisor.quiet configuration option Tom Hughes
2024-05-23 22:23 ` Junio C Hamano
2024-05-24  8:31   ` Tom Hughes
2024-05-24  9:09 ` [PATCH v2] " Tom Hughes
2024-05-24 18:06   ` Junio C Hamano [this message]
2024-05-25 10:10     ` Tom Hughes
2024-05-25 10:09   ` [PATCH v3] " Tom Hughes
2024-05-25  5:29 ` [PATCH] " Jeff King
2024-05-25 10:29   ` Tom Hughes
2024-05-29  9:36     ` Jeff King

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

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

  git send-email \
    --in-reply-to=xmqqa5kfkrsu.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=tom@compton.nu \
    /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.
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 NNTP newsgroup(s).