* [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs
@ 2014-09-14 8:30 David Aguilar
2014-09-14 16:20 ` Fabian Ruch
0 siblings, 1 reply; 5+ messages in thread
From: David Aguilar @ 2014-09-14 8:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Ensure that rev-parse --verify --quiet is silent when asked
about deleted reflog entries.
Signed-off-by: David Aguilar <davvid@gmail.com>
---
This verifies and depends on "refs: make rev-parse --quiet actually quiet".
t/t1503-rev-parse-verify.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
index 813cc1b..731c21c 100755
--- a/t/t1503-rev-parse-verify.sh
+++ b/t/t1503-rev-parse-verify.sh
@@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' '
test -z "$(cat error)"
'
+test_expect_success 'fails silently when using -q with deleted reflogs' '
+ ref=$(git rev-parse HEAD) &&
+ : >.git/logs/refs/test &&
+ git update-ref -m test refs/test "$ref" &&
+ git reflog delete --updateref --rewrite refs/test@{0} &&
+ test_must_fail git rev-parse --verify --quiet refs/test@{0} 2>error &&
+ test -z "$(cat error)"
+'
+
test_expect_success 'no stdout output on error' '
test -z "$(git rev-parse --verify)" &&
test -z "$(git rev-parse --verify foo)" &&
--
2.1.0.29.gf6d9003.dirty
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs
2014-09-14 8:30 [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs David Aguilar
@ 2014-09-14 16:20 ` Fabian Ruch
2014-09-14 18:54 ` David Aguilar
0 siblings, 1 reply; 5+ messages in thread
From: Fabian Ruch @ 2014-09-14 16:20 UTC (permalink / raw)
To: David Aguilar, Junio C Hamano; +Cc: git
Hi David,
On 09/14/2014 10:30 AM, David Aguilar wrote:
> Ensure that rev-parse --verify --quiet is silent when asked
> about deleted reflog entries.
>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> This verifies and depends on "refs: make rev-parse --quiet actually quiet".
>
> t/t1503-rev-parse-verify.sh | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
> index 813cc1b..731c21c 100755
> --- a/t/t1503-rev-parse-verify.sh
> +++ b/t/t1503-rev-parse-verify.sh
> @@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' '
> test -z "$(cat error)"
> '
>
> +test_expect_success 'fails silently when using -q with deleted reflogs' '
> + ref=$(git rev-parse HEAD) &&
> + : >.git/logs/refs/test &&
> + git update-ref -m test refs/test "$ref" &&
I'm just curious, why not simply
git branch test
?
> + git reflog delete --updateref --rewrite refs/test@{0} &&
> + test_must_fail git rev-parse --verify --quiet refs/test@{0} 2>error &&
Is it a shortcoming of the specification that it doesn't consider
whatever might be written to stdout? Is it acceptable that if the
git-rev-parse command succeeds, the error message from test_must_fail
will be written to the file "error" and, therefore, somewhat hidden from
the user running the tests?
> + test -z "$(cat error)"
test(1) comes with an option (-s) to perform such tests and test-lib.sh
defines test_must_be_empty which additionally outputs the given file's
contents if its not empty.
> +'
> +
> test_expect_success 'no stdout output on error' '
> test -z "$(git rev-parse --verify)" &&
> test -z "$(git rev-parse --verify foo)" &&
>
Kind regards,
Fabian
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs
2014-09-14 16:20 ` Fabian Ruch
@ 2014-09-14 18:54 ` David Aguilar
2014-09-15 18:17 ` Junio C Hamano
2014-09-15 18:25 ` Junio C Hamano
0 siblings, 2 replies; 5+ messages in thread
From: David Aguilar @ 2014-09-14 18:54 UTC (permalink / raw)
To: Fabian Ruch; +Cc: Junio C Hamano, git
On Sun, Sep 14, 2014 at 06:20:57PM +0200, Fabian Ruch wrote:
> Hi David,
>
> On 09/14/2014 10:30 AM, David Aguilar wrote:
> > Ensure that rev-parse --verify --quiet is silent when asked
> > about deleted reflog entries.
> >
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> > This verifies and depends on "refs: make rev-parse --quiet actually quiet".
> >
> > t/t1503-rev-parse-verify.sh | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh
> > index 813cc1b..731c21c 100755
> > --- a/t/t1503-rev-parse-verify.sh
> > +++ b/t/t1503-rev-parse-verify.sh
> > @@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' '
> > test -z "$(cat error)"
> > '
> >
> > +test_expect_success 'fails silently when using -q with deleted reflogs' '
> > + ref=$(git rev-parse HEAD) &&
> > + : >.git/logs/refs/test &&
> > + git update-ref -m test refs/test "$ref" &&
>
> I'm just curious, why not simply
>
> git branch test
> ?
Maybe it's a bad reason, but I wanted to replicate the behavior
that git stash expects -- it writes to a ref outside of
refs/heads/. I thought it'd be good to exercise that same
machinery since it will involve different code paths.
> > + git reflog delete --updateref --rewrite refs/test@{0} &&
> > + test_must_fail git rev-parse --verify --quiet refs/test@{0} 2>error &&
>
> Is it a shortcoming of the specification that it doesn't consider
> whatever might be written to stdout? Is it acceptable that if the
> git-rev-parse command succeeds, the error message from test_must_fail
> will be written to the file "error" and, therefore, somewhat hidden from
> the user running the tests?
Good point. The --quiet spec doesn't say anything about stdout,
but for this test it probably wouldn't hurt to capture both
stdout and stderr and assert emptiness.
I can reroll this patch so that 2>error becomes >error 2>&1.
> > + test -z "$(cat error)"
>
> test(1) comes with an option (-s) to perform such tests and test-lib.sh
> defines test_must_be_empty which additionally outputs the given file's
> contents if its not empty.
test_must_be_empty would be a good fit here. That said, none of
the other tests in this file use test_must_be_empty.
It might be worth doing a follow-up patch that converts all of the
tests in this file to use test_must_be_empty instead of
test -z "$(cat error)". I'll reroll.
Thanks,
--
David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs
2014-09-14 18:54 ` David Aguilar
@ 2014-09-15 18:17 ` Junio C Hamano
2014-09-15 18:25 ` Junio C Hamano
1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-09-15 18:17 UTC (permalink / raw)
To: David Aguilar; +Cc: Fabian Ruch, git
David Aguilar <davvid@gmail.com> writes:
> Good point. The --quiet spec doesn't say anything about stdout,
Please correct it while at it in the doc ;-)
I think I had to look it up in the documentation and then in code if
git rev-parse --verify --quiet "$object"
the right way to check if the object is a good name without output
when it is, and get diagnosis in an appropriate error message when
it isn't.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs
2014-09-14 18:54 ` David Aguilar
2014-09-15 18:17 ` Junio C Hamano
@ 2014-09-15 18:25 ` Junio C Hamano
1 sibling, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-09-15 18:25 UTC (permalink / raw)
To: David Aguilar; +Cc: Fabian Ruch, git
David Aguilar <davvid@gmail.com> writes:
>> > +test_expect_success 'fails silently when using -q with deleted reflogs' '
>> > + ref=$(git rev-parse HEAD) &&
>> > + : >.git/logs/refs/test &&
>> > + git update-ref -m test refs/test "$ref" &&
>>
>> I'm just curious, why not simply
>>
>> git branch test
>> ?
>
> Maybe it's a bad reason, but I wanted to replicate the behavior
> that git stash expects -- it writes to a ref outside of
> refs/heads/. I thought it'd be good to exercise that same
> machinery since it will involve different code paths.
I think that is a very sensible thing to do. Another reason to
avoid using "branch" when you care about what "update-ref" does is
that "branch" does more than what "update-ref" does.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-15 18:25 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-14 8:30 [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs David Aguilar
2014-09-14 16:20 ` Fabian Ruch
2014-09-14 18:54 ` David Aguilar
2014-09-15 18:17 ` Junio C Hamano
2014-09-15 18:25 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.