Re: [RFC PATCH 1/5] Add testcases for improved file collision conflict handling

From: Elijah Newren <newren@gmail.com>
Date: Mar 8, 2018, 17:51 UTC
Message-ID: <CABPp-BGO1m=OAbUS8RUKdqP+8p-DokzbjGa3TcigmTSSa_r3-Q@mail.gmail.com>
In-Reply-To: <20180308122523.14434-1-szeder.dev@gmail.com>

Sweet! Thanks for taking a look, and for spotting lots of improvements (and some really embarrassing errors). I'll keep the fixes queued up while waiting for other feedback. A few comments...

On Thu, Mar 8, 2018 at 4:25 AM, SZEDER Gábor <szeder.dev@gmail.com> wrote:

> This setup test is enormous, and the conditions for the combination of
> the two sides and the add/rename conflicts are somewhat distracting.
> I don't know how it could be structured better/shorter/clearer...  I
> couldn't come up with anything useful during lunch.

Yeah. :-( It's part attempt to test these conflict types much more thoroughly than other tests in the testsuite do, and part attempt to keep the test setup consistent between the types to reflect the fact that I'm consolidating the conflict resolution into a single function as well.

Two possible ideas:

  * Split the tests for "*_unrelated" files and "*_related" files into
separate tests (doubling the number of tests, but making each only
deal with half the files.  That would make each setup function about
half the size, though both check functions would be about as big as
the original.

  * Instead of programatically generated tests, just manually write
out the tests for each of the four combinations (rename/rename,
rename/add, add/rename, add/add).  That means four "copies" of fairly
similar functions (and possible greater difficulty keeping things
consistent if changes are requested), but does allow removal of the
three different if-statements and thus makes each one easier to
understand in isolation.

Doing both would be possible as well.

Personally, I'm much more in favor of the first idea than the second. I'm still kind of borderline about whether to make the change mentioned in the first idea, but if others feel that splitting would help a lot, I'm happy to look into either or both ideas.

Show 12 quoted lines

>> +                     cat <<EOF >>expected &&
>> +<<<<<<< HEAD
>> +modification
>> +=======
>> +more stuff
>> +yet more stuff
>> +>>>>>>> R^0
>> +EOF
>
> You could use 'cat <<-EOF ....' and indent the here doc with tabs, so
> it won't look so out-of-place.  Or even '<<-\EOF' to indicate that
> there is nothing to be expanded in the here doc.

I just learned two new things about heredocs; I've wanted both of those things in the past, but didn't even think to check if they were possible. Thanks for enlightening me.