Web lists-archives.com

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




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@xxxxxxxxx> 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.

>> +                     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.