Re: [PATCH v2] promisor-remote: add promisor.quiet configuration option

From: Junio C Hamano <gitster@pobox.com>
Date: May 24, 2024, 18:06 UTC
Message-ID: <xmqqa5kfkrsu.fsf@gitster.g>
In-Reply-To: <20240524090937.2448229-1-tom@compton.nu>

Tom Hughes <tom@compton.nu> writes:

Show 35 quoted lines

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