Re: [PATCH] merge_blobs: use strbuf instead of manually-sized mmfile_t

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 16, 2016, 21:27 UTC
Message-ID: <xmqqmvr0mjmc.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160216055043.GB28237@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 9 quoted lines

> Yeah, maybe. There were two reasons I avoided adding a test.
>
> One, I secretly hoped that by dragging my feet we could get consensus on
> just ripping out merge-tree entirely. ;)
>
> Two, I'm not sure what the test output _should_ be. I think this case is
> buggy. So we can check that we don't corrupt the heap, which is nice,
> but I don't like adding a test that doesn't test_cmp the expected output
> (and see my earlier comments re: thinking hard).

Three, I know the existence of the program is not more than "we could do something like this" illustration by Linus, and its output is in no way _designed_ to be so. We know today that it does not do add/add conflict correctly thanks to this thread, but if we are going to keep this program, we'd need to start from really designing what its behaviour _should_ be, before casting the desirable behaviour into stone with tests.

> But if we are going to keep it, maybe some exercise of the code is
> better than none.

So, "exercise" is better than none in the sense that it would validate that "the command does something without barfing", but I think there is a downside to casting its current behaviour as the expected output in stone, if we are going to keep it.