Re: [PATCH v3 01/10] receive.fsck.<msg-id> tests: remove dead code

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 27, 2018, 19:45 UTC
Message-ID: <87d0v85uoy.fsf@evledraar.gmail.com>
In-Reply-To: <xmqqva90mr2l.fsf@gitster-ct.c.googlers.com>

On Fri, Jul 27 2018, Junio C Hamano wrote:

Show 18 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Remove the setting of a receive.fsck.badDate config variable to
>> "ignore". This was added in efaba7cc77 ("fsck: optionally ignore
>> specific fsck issues completely", 2015-06-22) but never did anything,
>> presumably it was part of some work-in-progress code that never made
>> it into git.git.
>>
>> None of these tests will emit the "invalid author/committer line - bad
>> date" warning. The dates on the commit objects we're setting up are
>> not invalid.
>
> It is a timestamp somewhere mid February of 2009.  Perhaps the code
> is playing defensive against the lack of email address on the
> deliberately broken author line, i.e.
>
>     author Bugs Bunny 1234567890 +0000
>     committer Bugs Bunny <bugs@bun.ni> 1234567890 +0000

This is covered by the "missingEmail" part of the test, but there's nothing wrong with the timestamp itself.

I doubt Johannes remembers why he did this almost a decade ago, but it looks to me like he was working on some test where the date was also bad, and never finished it. There's no point in having that "badDate" now.

Show 7 quoted lines

> in case the parser punted and failed to parse that timestamp
> correctly?  IOW, the above _could_ be a commit written by "Bugs
> Bunny 1234567890" with missing e-mail and missing timestamp.
>
> So I dunno.  It won't break the test with today's system if we
> removed this config, but with an updated parser from the next year,
> it may start to break.

I still think it makes sense to remove this particular thing. Let's add exhaustive tests for all this fsck.* stuff in another series, but no point in testing for arbitrary fsck errors that aren't going to be triggered in unrelated tests.

Show 19 quoted lines

>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>  t/t5504-fetch-receive-strict.sh | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
>> index 49d3621a92..e1f8768094 100755
>> --- a/t/t5504-fetch-receive-strict.sh
>> +++ b/t/t5504-fetch-receive-strict.sh
>> @@ -149,8 +149,6 @@ test_expect_success 'push with receive.fsck.missingEmail=warn' '
>>  	git --git-dir=dst/.git branch -D bogus &&
>>  	git --git-dir=dst/.git config --add \
>>  		receive.fsck.missingEmail ignore &&
>> -	git --git-dir=dst/.git config --add \
>> -		receive.fsck.badDate warn &&
>>  	git push --porcelain dst bogus >act 2>&1 &&
>>  	! grep "missingEmail" act
>>  '