From: Michael J Gruber <git@drmicha•warpmail.net>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, Junio C Hamano <gitster@pobox•com>
Subject: Re: [PATCH] t3200: test branch creation with -v
Date: Tue, 13 Sep 2011 14:12:52 +0200 [thread overview]
Message-ID: <4E6F48C4.3030407@drmicha.warpmail.net> (raw)
In-Reply-To: <20110913035724.GA4828@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 13.09.2011 05:57:
> On Sat, Sep 10, 2011 at 03:29:43PM +0200, Michael J Gruber wrote:
>
>> Jeff King venit, vidit, dixit 09.09.2011 21:43:
>>> On Fri, Sep 09, 2011 at 09:40:59PM +0200, Michael J Gruber wrote:
>>>
>>>> +test_expect_success 'git branch -v t should work' ' + git branch
>>>> -v t && + test .git/refs/heads/t &&
>>>
>>> test -f ?
>>>
>>> Also, don't we have test_path_is_file which yields slightly prettier
>>> output (and maybe some portability benefits; I don't remember)?
>>>
>>>> + git branch -d t && + test ! -f .git/refs/heads/t
>>>
>>> Ditto for 'test_path_is_missing' here.
>>>
>>> -Peff
>>
>> Well, I tried to follow the surrounding style. That t3200 could benefit
>> from some attention, though, which I did not want to mix in with the
>> issue at hand.
>
> The "test_path_is_file" thing is style. But not using "test -f" is just
> wrong; you are testing "is .git/refs/heads/t an empty string?" which is
> useless.
>
> You want this on top of what's in mg/branch-list:
Yes, sorry. How did I miss that?
I'd prefer your style anyway, but also prefer changing t3200 in one go.
>
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index c466b20..b513115 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -100,14 +100,14 @@ test_expect_success 'git branch -m q r/q should fail when r exists' '
>
> test_expect_success 'git branch -v -d t should work' '
> git branch t &&
> - test .git/refs/heads/t &&
> + test -f .git/refs/heads/t &&
> git branch -v -d t &&
> test ! -f .git/refs/heads/t
> '
>
> test_expect_success 'git branch -v -m t s should work' '
> git branch t &&
> - test .git/refs/heads/t &&
> + test -f .git/refs/heads/t &&
> git branch -v -m t s &&
> test ! -f .git/refs/heads/t &&
> test -f .git/refs/heads/s &&
> @@ -116,7 +116,7 @@ test_expect_success 'git branch -v -m t s should work' '
>
> test_expect_success 'git branch -m -d t s should fail' '
> git branch t &&
> - test .git/refs/heads/t &&
> + test -f .git/refs/heads/t &&
> test_must_fail git branch -m -d t s &&
> git branch -d t &&
> test ! -f .git/refs/heads/t
> @@ -124,7 +124,7 @@ test_expect_success 'git branch -m -d t s should fail' '
>
> test_expect_success 'git branch --list -d t should fail' '
> git branch t &&
> - test .git/refs/heads/t &&
> + test -f .git/refs/heads/t &&
> test_must_fail git branch --list -d t &&
> git branch -d t &&
> test ! -f .git/refs/heads/t
>
> I suspect you didn't notice the bogosity before because those are just
> confirming the precondition that "git branch" actually created the file.
>
> -Peff
next prev parent reply other threads:[~2011-09-13 12:13 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-02 17:17 [RFC] branch: list branches by single remote Michael Schubert
2011-08-04 4:06 ` Jeff King
2011-08-16 13:37 ` Michael J Gruber
2011-08-16 14:19 ` Michael Schubert
2011-08-16 15:14 ` Jeff King
2011-08-24 15:14 ` Michael Schubert
2011-08-24 15:37 ` Michael J Gruber
2011-08-24 16:02 ` Michael Schubert
2011-08-24 18:34 ` Junio C Hamano
2011-08-25 2:31 ` Thiago Farina
2011-08-25 18:02 ` Junio C Hamano
2011-08-25 8:29 ` Michael J Gruber
2011-08-25 8:30 ` [PATCH 0/5] RFC: patterns for branch list Michael J Gruber
2011-08-25 8:30 ` [PATCH 1/5] branch: allow pattern arguments Michael J Gruber
2011-08-25 17:54 ` Jeff King
2011-08-25 18:32 ` Junio C Hamano
2011-08-25 19:29 ` Junio C Hamano
2011-08-25 8:30 ` [PATCH 2/5] branch: introduce --list argument Michael J Gruber
2011-08-25 18:34 ` Junio C Hamano
2011-08-25 19:52 ` Junio C Hamano
2011-08-25 8:30 ` [PATCH 3/5] t6040; test branch -vv Michael J Gruber
2011-08-25 8:30 ` [PATCH 4/5] branch: restructure -v vs. -vv Michael J Gruber
2011-08-25 19:02 ` Junio C Hamano
2011-08-25 8:30 ` [PATCH 5/5] branch: give patchsame count with -vvv Michael J Gruber
2011-08-25 17:53 ` [PATCH 0/5] RFC: patterns for branch list Jeff King
2011-08-26 8:30 ` Michael J Gruber
2011-08-26 16:55 ` Junio C Hamano
2011-09-07 19:53 ` Jeff King
2011-09-08 9:20 ` Michael J Gruber
2011-08-26 14:05 ` [PATCHv2 0/5] " Michael J Gruber
2011-08-26 14:05 ` [PATCHv2 1/5] t6040: test branch -vv Michael J Gruber
2011-08-26 14:05 ` [PATCHv2 2/5] git-tag: introduce long forms for the options Michael J Gruber
2011-08-26 17:11 ` Junio C Hamano
2011-08-28 14:03 ` Michael J Gruber
2011-08-26 14:05 ` [PATCHv2 3/5] git-branch: introduce missing " Michael J Gruber
2011-08-26 17:13 ` Junio C Hamano
2011-08-28 14:05 ` Michael J Gruber
2011-08-26 14:05 ` [PATCHv2 4/5] branch: introduce --list option Michael J Gruber
2011-08-26 17:43 ` Junio C Hamano
2011-08-28 14:37 ` Michael J Gruber
2011-09-07 19:56 ` Jeff King
2011-09-08 9:24 ` Michael J Gruber
2011-09-08 14:25 ` [PATCHv4 0/5] mg/branch-list amendment Michael J Gruber
2011-09-08 14:25 ` [PATCHv4 4/5] branch: introduce --list option Michael J Gruber
2011-09-08 14:25 ` [PATCHv4 5/5] branch: allow pattern arguments Michael J Gruber
2011-09-08 21:03 ` [PATCHv2 4/5] branch: introduce --list option Junio C Hamano
2011-09-08 21:11 ` Jeff King
2011-09-08 21:17 ` Junio C Hamano
2011-09-09 1:08 ` Jeff King
2011-09-09 6:54 ` Michael J Gruber
2011-09-09 16:02 ` Junio C Hamano
2011-09-09 19:29 ` Michael J Gruber
2011-09-09 19:30 ` Jeff King
2011-09-09 19:40 ` [PATCH] t3200: test branch creation with -v Michael J Gruber
2011-09-09 19:43 ` Jeff King
2011-09-09 19:45 ` Jeff King
2011-09-10 13:29 ` Michael J Gruber
2011-09-13 3:57 ` Jeff King
2011-09-13 12:12 ` Michael J Gruber [this message]
2011-09-13 16:13 ` [PATCH] t3200: clean up checks for file existence Jeff King
2011-09-13 17:13 ` Junio C Hamano
2011-09-13 17:16 ` Jeff King
2011-08-26 14:05 ` [PATCHv2 5/5] branch: allow pattern arguments Michael J Gruber
2011-08-26 18:45 ` Junio C Hamano
2011-08-28 14:54 ` [PATCHv3 0/5] patterns for branch list Michael J Gruber
2011-08-28 14:54 ` [PATCHv3 1/5] t6040: test branch -vv Michael J Gruber
2011-08-28 14:54 ` [PATCHv3 2/5] git-tag: introduce long forms for the options Michael J Gruber
2011-08-28 14:54 ` [PATCHv3 3/5] git-branch: introduce missing " Michael J Gruber
2011-08-28 14:54 ` [PATCHv3 4/5] branch: introduce --list option Michael J Gruber
2011-08-29 5:55 ` Junio C Hamano
2011-08-29 6:35 ` Michael J Gruber
2011-08-29 6:51 ` Junio C Hamano
2011-08-28 14:54 ` [PATCHv3 5/5] branch: allow pattern arguments Michael J Gruber
2011-09-06 13:10 ` Michael Schubert
2011-09-06 14:21 ` Michael J Gruber
2011-09-06 14:26 ` Sverre Rabbelier
2011-09-06 16:11 ` Michael J Gruber
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=4E6F48C4.3030407@drmicha.warpmail.net \
--to=git@drmicha$(echo .)warpmail.net \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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