Re: Incremental updates to What's cooking

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 29, 2012, 08:39 UTC
Message-ID: <7vbooiuj6z.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4F4DD5C1.60604@in.waw.pl>

Zbigniew Jędrzejewski-Szmek  <zbyszek@in.waw.pl> writes:

Show 49 quoted lines

> 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?

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.