Re: [PATCH v2 10/16] tag: change misleading --list <pattern> documentation

From: Jeff King <peff@peff.net>
Date: Mar 22, 2017, 22:36 UTC
Message-ID: <20170322223600.huyel6j5wl644ddf@sigill.intra.peff.net>
In-Reply-To: <xmqqtw6lgd82.fsf@gitster.mtv.corp.google.com>

On Wed, Mar 22, 2017 at 03:26:21PM -0700, Junio C Hamano wrote:

Show 20 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 
> > of things you think we should be putting in the test suite. I.e.
> > should the tests be:
> >
> > a) Only be a collection of invocations of git we'd be comfortable
> > showing to someone as "this works, and this is how you should do it",
> > or things that explicitly fail marked with test_must_fail.
> >
> > b) or a) && also various surprising combinations of things we don't
> > necessarily want to encourage or even support in the future, but which
> > are in there so if we change them, we at least know our change changed
> > something that worked before.
> 
> I am strongly inclined to (a).  If we cannot decide when we designed
> the feature, and we anticipate that we may want to change it later,
> then documenting the choice in a test or two may be a way to remind
> the choice we happened to have made, but in general I do not think
> we want to promise (to ourselves) more than what we are willing to
> commit to.

I've occasionally[1] added tests that are "what we happen to produce now", but I almost always mark them with a comment either in the test script or in the commit message. What I'm _most_ concerned about is a developer later breaking the test, but being unsure if they were breaking some real-world case (and not being able to find clues in the history).

A secondary concern would be people using the test snippets as guidance on what is normal or encouraged.

So I could live with these patches, but I'd prefer to see a comment somewhere. And I think I'd have a slight inclination to just stick to (a) in the first place, unless there is a really good reason to cover the test (like that we do not care between behaviors X and Y, but we need to check that it does one of them, and not Z).

-Peff

[1] E.g., see the comment in t3204 from a356e8e2a (t3204: test
    git-branch @-expansion corner cases, 2017-03-02).