# [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs 5 messages from 2014-09-14 to 2014-09-15. Participants: David Aguilar, Fabian Ruch, Junio C Hamano. Thread: https://gitlist.dev/t/37561 ## David Aguilar, 2014-09-14 08:30 Subject: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs Message-ID: <1410683442-74523-1-git-send-email-davvid@gmail.com> URL: https://gitlist.dev/e/1410683442-74523-1-git-send-email-davvid%40gmail.com ``` Ensure that rev-parse --verify --quiet is silent when asked about deleted reflog entries. Signed-off-by: David Aguilar --- 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 ``` ## Fabian Ruch, 2014-09-14 16:20 Subject: Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs Message-ID: <5415C069.9000702@gmail.com> URL: https://gitlist.dev/e/5415C069.9000702%40gmail.com In-Reply-To: <1410683442-74523-1-git-send-email-davvid@gmail.com> ``` 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 > --- > 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 ``` ## David Aguilar, 2014-09-14 18:54 Subject: Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs Message-ID: <20140914185403.GA93515@gmail.com> URL: https://gitlist.dev/e/20140914185403.GA93515%40gmail.com In-Reply-To: <5415C069.9000702@gmail.com> ``` 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 > > --- > > 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 ``` ## Junio C Hamano, 2014-09-15 18:17 Subject: Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs Message-ID: URL: https://gitlist.dev/e/xmqqy4tk7npg.fsf%40gitster.dls.corp.google.com In-Reply-To: <20140914185403.GA93515@gmail.com> ``` David Aguilar 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. ``` ## Junio C Hamano, 2014-09-15 18:25 Subject: Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs Message-ID: URL: https://gitlist.dev/e/xmqqppew7nbt.fsf%40gitster.dls.corp.google.com In-Reply-To: <20140914185403.GA93515@gmail.com> ``` David Aguilar 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. ```