From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 29 Feb 2012 13:50:42 GMT
Subject: Re: Incremental updates to What's cooking
Message-ID: <4F4E2D32.9030209@in.waw.pl>
In-Reply-To: <7vbooiuj6z.fsf@alter.siamese.dyndns.org>

On 02/29/2012 09:39 AM, Junio C Hamano wrote:
> Zbigniew Jędrzejewski-Szmek<zbyszek@in.waw.pl>  writes:
>
>> On 02/28/2012 07:53 AM, Junio C Hamano wrote:
>>
>>> * zj/diff-stat-dyncol (2012-02-27) 11 commits
>>>    - diff --stat: add config option to limit graph width
>>>    - diff --stat: enable limiting of the graph part
>>>    - diff --stat: add a test for output with COLUMNS=40
>>>    - diff --stat: use a maximum of 5/8 for the filename part
>>>    - merge --stat: use the full terminal width
>>>    - log --stat: use the full terminal width
>>>    - show --stat: use the full terminal width
>>>    - diff --stat: use the full terminal width
>>>    - diff --stat: tests for long filenames and big change counts
>>>    - t4014: addtional format-patch test vectors
>>>    - Merge branches zj/decimal-width, zj/term-columns and jc/diff-stat-scaler
>>>
>>> I resurrected the additional tests for format-patch from an earlier round,
>>> as it illustrates the behaviour change brought by "5/8 split" very well.
>> Hi,
>> the resurrected tests are partly duplicated in 4052-stat-output.sh:
>>
>> t4014:
>> ok 75 - small change with long name gives more space to the name
>> ok 76 - a long name is given more room when the bar is short
>> ok 77 - format patch --stat-width=width works with long name        *
>> ok 78 - format patch --stat=...,name-width with long name           *
>> ok 79 - format patch --stat-name-width with long name               *
>> ok 81 - format patch graph part width                               *
>> ok 82 - format patch ignores COLUMNS                                *
>> ok 83 - format patch --stat=width with big change                   *
>> ok 84 - format patch --stat-width=width with big change             *
>> ok 85 - partition between long name and big change is more balanced
>>
>> t4052:
>> ok 3 - format-patch graph width defaults to 80 columns
>> ok 4 - format-patch --stat=width with long name
>> ok 5 - format-patch --stat-width=width with long name
>> ok 6 - format-patch --stat=...,name-width with long name
>> ok 7 - format-patch --stat-name-width with long name
>> ok 24 - format-patch ignores too many COLUMNS (big change)
>> ok 28 - format-patch ignores not enough COLUMNS (big change)
>> ok 29 - format-patch ignores statGraphWidth config
>> ok 36 - format-patch --stat=width with big change
>> ok 37 - format-patch --stat-width=width with big change
>> ok 38 - format-patch --stat-graph--width with big change
>> ok 49 - format-patch --stat=width with big change and long name
>> ok 53 - format-patch ignores COLUMNS (long filename)
>>
>> The ones with * are duplicated exactly. They tests run very fast, but
>> maybe the duplicated ones should be culled.
>
> Yeah, probably we should de-dup them.
>
> Compare the behaviour change shown for t4052 and for t4014 by 119c07bf.
> Which one more obviously show the effect of the code change to allow the
> reader judge if the behaviour change is going in a good direction?
t4014 it seems, but mostly because of more descriptive test names.
But this is only true for the ones without *, I think. So the ones with 
* can be deleted without losing this advantage.

The ones with * are very similar in t4014 and t4052.

> The style used in t4052 only changes expect_failure to expect_success, and
> the reader has to accept the judgement of the person who wrote the test
> vector and declared "this is the _right_ output!".  The way t4014, taken
> from your earlier round, shows the behaviour change shows how the
> expectation changes from the old behaviour to the new one, and the reader
> can see and decide which one is giving a better output.
>
> Actually, the whole reason I didn't notice duplicates in 4052 was because
> of the above X-<.
>
> If we remove duplicates, will 4052 become empty?  It would be really nice
> if we do not have to add a new test script for this series, and instead
> add necessary new tests to existing scripts.
t4052 tests show, log, merge, diff and format-patch with basically the 
same commands. Separating the tests into different files would require 
duplicating a lot of setup code. OTOH, t4014 is only about format-patch, 
so the other ones don't fit. I thought it would be better to create a 
new file.

-
Zbyszek