From: Luke Diamand <luke@diamand.org>
Date: Mon, 21 Sep 2015 07:47:32 GMT
Subject: Re: [PATCH v3 1/2] git-p4: add test case for "Translation of file content failed" error
Message-ID: <55FFB614.7000908@diamand.org>
In-Reply-To: <CAPig+cSFbYzLi8daS55B_sdvDPW4q6yOC5Hm75+Rmr-s1aFRHw@mail.gmail.com>

On 20/09/15 23:29, Eric Sunshine wrote:
> On Sun, Sep 20, 2015 at 5:34 PM, Lars Schneider
> <larsxschneider@gmail.com> wrote:
>
>> What is the preferred way by the Git community? Combine patch and
>> test in one commit or a patch commit followed by a test commit? I
>> would prefer to have everything in one commit.
>
> If the tests are in a separate patch, Junio seems to prefer adding
> them after the problem is fixes; the idea being that tests are added
> to ensure that some future change doesn't break the feature, as
> opposed to showing that your patch fixes a bug.
>
> Whether or not to combine the fix with the new tests often depends
> upon the length of the patches and how easy or hard it is to review
> them. In this case, the fix itself is fairly short, but the tests are
> slightly lengthy, so there may not be a clear cut answer. As a
> reviewer, I tend to prefer smaller patches, however, this situation
> doesn't demand it, so use your best judgment.

I think in the past we have a test added that demonstrates the problem, 
with "test_expect_failure", followed by the fix, which also flips the 
test to "test_expect_success".

Luke