Re: [PATCH] for-each-repo: do nothing on empty config

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2021, 21:40 UTC
Message-ID: <xmqqv9c9pwv0.fsf@gitster.c.googlers.com>
In-Reply-To: <xmqq1rexssqp.fsf@gitster.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 27 quoted lines

> Derrick Stolee <stolee@gmail.com> writes:
>
>>> I wonder if "false" or "exit 1" would fit the bill.  In any case, a
>>> comment may help, perhaps?
>>> 
>>> 	test_expect_success 'do nothing and succeed on empty/missing config' '
>>> 		# if this runs even once, "false" ensures a failure
>>> 		git for-each-repo --config=bogus.config -- false
>>> 	'
>>
>> I can add a comment, but keep in mind that this example would run the
>> subcommand as "git false". This isn't intended as an arbitrary script
>> runner, but a "please run the same Git command on a list of repos".
>
> Ah, that is a good point.
>
> The comment needs to explain:
>
> 	# the command fails if it attempts to run even once because
> 	# 'git false' does not exist
>
> and at that point, it does not have to be spelled 'false'.  It could
> be 'no-such-git-subcommand' (and I wonder if that makes the comment
> unnecessary).
>
> That reminds me.  If I have ~/bin/git-false and ~/bin on my $PATH,
> would this test fail to catch breakage?

Yes, I think $PATH in the test environment starts from the original $PATH and modified only by prepending, so my ~/bin/git-false would kick in. We cannot reliably depend on the absence of a subcommand.

We can instead use

	# the whole thing would fail if for-each-ref iterated even
	# once, because 'git help --no-such-option' would fail
	git for-each-ref --config=<var> -- help --no-such-option

and I think that would be much more reliable; if an invocation of "git help" inside our test suite fails to refer to the "git help" from the version of Git being tested, we already have a serious problem.

Thanks.