Re: [GSoC][PATCH] tag: Make "git tag --contains <id>" less chatty if <id> is invalid

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2018, 23:29 UTC
Message-ID: <xmqqinar1bq2.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAGZ79kbXXN_nxFZUorsbFHcg4-8H7ZnAOnaViUrnP4r4u6ThhQ@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 9 quoted lines

>> diff --git a/t/t7013-tag-contains.sh b/t/t7013-tag-contains.sh
>> new file mode 100755
>> index 000000000..65119dada
>> --- /dev/null
>> +++ b/t/t7013-tag-contains.sh
>
> Thanks for adding the tests into a new file instead of putting it somewhere
> where it is already convenient. (We have too many of those "just add it there
> as it is easiest to fit in")

Careful, as that cuts both ways. We want to strongly encourage people to see if there is already a place that is a good enough fit for new tests before adding small test scripts randomly to consume the test serial numbers and test process start-up cost. Only when there is nowhere appropriate, we do want to add. And if this covers both tag and branch, then a new script may be appropriate but it shouldn't limit its future enhancement (to test 'git branch') by having 'tag' to pretend that this file must be limited to 'git tag'.

Show 8 quoted lines

> So I'd expect the return code to be 0 (if we don't care) or 1
> (if we do care), in the case of 1, we shall write:
>
>   test_must_fail git tag --contains ... &&
>   grep ....
>
> (A long way of hinting at the test_must_fail test function,
> that lives in t/test-lib-functions.sh)

If you are looking at error stream, it is very likely that you would want to study test_i18ngrep and use it, as errors are fair game for i18n (and possibly coloring, which is a near-by topic).