git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: Christian Couder <christian.couder@gmail.com>,
	git <git@vger.kernel.org>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Subject: Re: [PATCH v3 7/8] branch.c: use 'ref-filter' APIs
Date: Mon, 24 Aug 2015 10:31:41 -0700	[thread overview]
Message-ID: <xmqqtwro4mpu.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAOLa=ZSwU94-JgAdw-xoL5mDNVL8nsbuBCD-MhN3H+m1gFD9gQ@mail.gmail.com> (Karthik Nayak's message of "Sat, 22 Aug 2015 23:20:32 +0530")

Karthik Nayak <karthik.188@gmail.com> writes:

>>>  test_expect_success 'git branch shows badly named ref' '
>>>         cp .git/refs/heads/master .git/refs/heads/broken...ref &&
>>>         test_when_finished "rm -f .git/refs/heads/broken...ref" &&
>>> -       git branch >output &&
>>> +       git branch 2>output &&
>>>         grep -e "broken\.\.\.ref" output
>>>  '
>>
>> Maybe the test could be renamed to 'git branch warns about badly named
>> ref' and maybe you could also check that "broken\.\.\.ref" is not
>> printed on stdout.
>
> The name change sounds reasonable, do we really need to check for it in the
> stdout?

I think Christian meant "we should check in both", i.e.

	git branch >output 2>error &&
        grep "broken\.\.\.ref" error &&
        ! grep "broken\.\.\.ref" output

or something like that.  That way, you are effectively specifying in
the test that badly named refs must not be included in the output,
so somebody who changes that in the future needs to justify why
including them in the output is a good idea when updating the test.

  reply	other threads:[~2015-08-24 17:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-22  6:51 [PATCH v3 0/8] port the filtering part of ref-filter to branch.c Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 1/8] branch: refactor width computation Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 2/8] branch: bump get_head_description() to the top Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 3/8] branch: roll show_detached HEAD into regular ref_list Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 4/8] branch: move 'current' check down to the presentation layer Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 5/8] branch: drop non-commit error reporting Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 6/8] branch.c: use 'ref-filter' data structures Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 7/8] branch.c: use 'ref-filter' APIs Karthik Nayak
2015-08-22 15:51   ` Christian Couder
2015-08-22 17:50     ` Karthik Nayak
2015-08-24 17:31       ` Junio C Hamano [this message]
2015-08-24 18:10         ` Karthik Nayak
2015-08-22  6:51 ` [PATCH v3 8/8] branch: add '--points-at' option Karthik Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtwro4mpu.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).