Re: [PATCH] t9001: Fix test prerequisites

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 29, 2010, 21:38 UTC
Message-ID: <7vhbdws1ht.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1293656551-5463-1-git-send-email-robbat2@gentoo.org>

"Robin H. Johnson" <robbat2@gentoo.org> writes:

Show 6 quoted lines

> Add in missing Perl prerequisites for new tests of send-email.
>
> Signed-off-by: Robin H. Johnson <robbat2@gentoo.org>
> ---
>  t/t9001-send-email.sh |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Thanks.

Show 22 quoted lines

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 1dc4a92..3271426 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1135,7 +1135,7 @@ test_expect_success $PREREQ '--8bit-encoding also treats subject' '
>  # Note that the patches in this test are deliberately out of order; we
>  # want to make sure it works even if the cover-letter is not in the
>  # first mail.
> -test_expect_success 'refusing to send cover letter template' '
> +test_expect_success $PREREQ 'refusing to send cover letter template' '
>  	clean_fake_sendmail &&
>  	rm -fr outdir &&
>  	git format-patch --cover-letter -2 -o outdir &&
> @@ -1151,7 +1151,7 @@ test_expect_success 'refusing to send cover letter template' '
>  	test -z "$(ls msgtxt*)"
>  '
>  
> -test_expect_success '--force sends cover letter template anyway' '
> +test_expect_success $PREREQ '--force sends cover letter template anyway' '
>  	clean_fake_sendmail &&
>  	rm -fr outdir &&
>  	git format-patch --cover-letter -2 -o outdir &&

This however makes me wonder (Robin, the following is primarily meant for Ævar to whom 57cd35e (t/t9001-send-email.sh: change from skip_all=* to prereq skip, 2010-08-13) is credited, and not a complaint to your patch at all, but you are welcome to comment on it if you feel like).

Everything in this test seem to require $PREREQ now (test_expect_success always takes the 3-parameter form). Does it suggest that we might want to allow tests to define a "global prerequisite", e.g.

   GIT_TESTS_PREREQ="PERL"
   export GIT_TESTS_PREREQ

and make the traditional 2-parameter test_expect_success without an explicit prerequisite take notice? Would it let us not have to worry about this kind of breakages? Or is 9001 a very tiny minority oddball that must have $PREREQ everywhere and such a test framework feature would be an overkill?

More importantly, 9001 is all about send-email and we know upfront that we want to skip everything when PERL prerequisite is not met. Why not a simple

	if ! test_have_prereq PERL
        then
        	test_done
	fi

insufficient?

Who cares "skipped statistics"? When one does not care about testing send-email at all, why should one care how many tests on that program are skipped? I personally do not think this is worth the trouble, and am very close (showing two fingers almost touching) to suggest reverting 57cd35e.

Please convince me otherwise.