From: Richard Hansen <hansenr@google.com>
Date: Thu, 05 Jan 2017 15:41:58 GMT
Subject: Re: [PATCH 2/4] t7610: make tests more independent and debuggable
Message-ID: <3f0222d0-fc61-ac41-3b1d-7f506fcb1f61@scientician.org>
In-Reply-To: <CAGZ79kb9UvV6AaRLBP5OzyRtTTXarRZRZDMd_1k5n9CrgbVr5A@mail.gmail.com>

On 2017-01-04 15:27, Stefan Beller wrote:
> On Tue, Jan 3, 2017 at 4:50 PM, Richard Hansen <hansenr@google.com> wrote:
>> If a test fails it might leave the repository in a strange state.  Add
>> 'git reset --hard' at the beginning of each test to increase the odds
>> of passing when an earlier test fails.
>
> So each test is cleaning up the previous test, which *may* confuse
> a reader ("how is the reset --hard relevant for this test? Oooh it's
> just a cleanup").
>
> We could put it another way by having each test itself make clean
> up after itself via
>
>   test_when_finished "git reset --hard" &&
>   ..
>
> at the beginning of each test.
> This would produce the same order of operations, i.e. a
> reset run between each test, but semantically tells the reader
> that the reset is part of the current test cleaning up after itself,
> as "reset" is operation for this particular test to cleanup.
> Does that make sense?

I like that idea; thanks for the suggestion.  I'll cook up a reroll.

>>
>> Also use test-specific branches to avoid interfering with later tests
>> and to make the tests easier to debug.
>
> That sounds great!
> Though in the code I only spot one occurrence for
>
> +       git checkout -b test$test_count branch1 &&
>
> so maybe that could be part of the first patch in the series?

There are two; the other is buried in the change for the 'mergetool on 
file in parent dir' test.

Thanks for the review,
Richard


>
> Thanks,
> Stefan