Re: [PATCH v2] pull: handle --log=<n>

From: Junio C Hamano <gitster@pobox.com>
Date: May 19, 2015, 23:14 UTC
Message-ID: <CAPc5daUF_yotXVf_=CFz_q1GBXVdq8pJxeuNuv=BAC54TJVT9w@mail.gmail.com>
In-Reply-To: <1432074626.14498.16.camel@kaarsemaker.net>

On Tue, May 19, 2015 at 3:30 PM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:

Show 19 quoted lines

> On di, 2015-05-19 at 15:10 -0700, Junio C Hamano wrote:
>> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>>
>> > I've actually done it differently while implementing:
>> >
>> > 1) Make test_commit recognize --tags and stop creating tags unless
>> >    specified
>> > 2) while ! prove --state=save,failed {
>> >        Find and fix tests that now need --tags
>> >    }
>>
>> That was what I feared.  The result of that process is the hardest
>> to reason about and review.
>>
>
> I'm not quite sure I understand what you're trying to say. You seem to
> be worried that there will be silent successes for tests that should
> fail after N/N if I take the proposed approach. I have no idea how that
> could happen though.

You are placing too much faith in the tests. We would want to see a conversion that is reasonable and indeed reasoned by eyeballing the change for correctness.

Besides, there are different build options and test configurations. Even if you run with EXPENSIVE defined, the only thing we can say without validating the changes is the test happened to have passed. There is no assurance that the result of your change will catch the same breakage, if inttroduced later to the code (not tests), as the current unmodified tests will. Two mistakes may happen to hide the breakage in tests. Taking to the extreme, if you replace all the test bodies in test_expect_success with "true", the result of such a rewrite will still pass all the existting tests, and the only way you can say "That's absurd" is because you compare what the existing tests do and a single "true" would do and realize that they do vastly different things. Your "while ! prove ..." method does not give me much more faith than such a "rewrite everything to true without looking" method.