{"thread":{"id":"37561","subject":"[PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs","startedAt":"2014-09-14T08:30:42Z","lastAt":"2014-09-15T18:25:42Z","messageCount":5,"participants":["David Aguilar","Fabian Ruch","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"249390","messageId":"1410683442-74523-1-git-send-email-davvid@gmail.com","threadId":"37561","inReplyTo":null,"subject":"[PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-14T08:30:42Z","receivedAt":"2014-09-14T08:30:42Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Ensure that rev-parse --verify --quiet is silent when asked\nabout deleted reflog entries.\n\nSigned-off-by: David Aguilar \n---\nThis verifies and depends on \"refs: make rev-parse --quiet actually quiet\".\n\n t/t1503-rev-parse-verify.sh | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\nindex 813cc1b..731c21c 100755\n--- a/t/t1503-rev-parse-verify.sh\n+++ b/t/t1503-rev-parse-verify.sh\n@@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' '\n \ttest -z \"$(cat error)\"\n '\n \n+test_expect_success 'fails silently when using -q with deleted reflogs' '\n+\tref=$(git rev-parse HEAD) &&\n+\t: >.git/logs/refs/test &&\n+\tgit update-ref -m test refs/test \"$ref\" &&\n+\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n+\ttest_must_fail git rev-parse --verify --quiet refs/test@{0} 2>error &&\n+\ttest -z \"$(cat error)\"\n+'\n+\n test_expect_success 'no stdout output on error' '\n \ttest -z \"$(git rev-parse --verify)\" &&\n \ttest -z \"$(git rev-parse --verify foo)\" &&\n-- \n2.1.0.29.gf6d9003.dirty\n"},{"id":"249399","messageId":"5415C069.9000702@gmail.com","threadId":"37561","inReplyTo":"1410683442-74523-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-09-14T16:20:57Z","receivedAt":"2014-09-14T16:20:57Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"Hi David,\n\nOn 09/14/2014 10:30 AM, David Aguilar wrote:\n> Ensure that rev-parse --verify --quiet is silent when asked\n> about deleted reflog entries.\n> \n> Signed-off-by: David Aguilar \n> ---\n> This verifies and depends on \"refs: make rev-parse --quiet actually quiet\".\n> \n> t/t1503-rev-parse-verify.sh | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\n> index 813cc1b..731c21c 100755\n> --- a/t/t1503-rev-parse-verify.sh\n> +++ b/t/t1503-rev-parse-verify.sh\n> @@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' '\n> \ttest -z \"$(cat error)\"\n> '\n> \n> +test_expect_success 'fails silently when using -q with deleted reflogs' '\n> +\tref=$(git rev-parse HEAD) &&\n> +\t: >.git/logs/refs/test &&\n> +\tgit update-ref -m test refs/test \"$ref\" &&\n\nI'm just curious, why not simply\n\n git branch test\n\n?\n\n> +\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n> +\ttest_must_fail git rev-parse --verify --quiet refs/test@{0} 2>error &&\n\nIs it a shortcoming of the specification that it doesn't consider\nwhatever might be written to stdout? Is it acceptable that if the\ngit-rev-parse command succeeds, the error message from test_must_fail\nwill be written to the file \"error\" and, therefore, somewhat hidden from\nthe user running the tests?\n\n> +\ttest -z \"$(cat error)\"\n\ntest(1) comes with an option (-s) to perform such tests and test-lib.sh\ndefines test_must_be_empty which additionally outputs the given file's\ncontents if its not empty.\n\n> +'\n> +\n> test_expect_success 'no stdout output on error' '\n> \ttest -z \"$(git rev-parse --verify)\" &&\n> \ttest -z \"$(git rev-parse --verify foo)\" &&\n> \n\nKind regards,\n Fabian\n"},{"id":"249403","messageId":"20140914185403.GA93515@gmail.com","threadId":"37561","inReplyTo":"5415C069.9000702@gmail.com","subject":"Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-09-14T18:54:04Z","receivedAt":"2014-09-14T18:54:04Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, Sep 14, 2014 at 06:20:57PM +0200, Fabian Ruch wrote:\n> Hi David,\n> \n> On 09/14/2014 10:30 AM, David Aguilar wrote:\n> > Ensure that rev-parse --verify --quiet is silent when asked\n> > about deleted reflog entries.\n> > \n> > Signed-off-by: David Aguilar \n> > ---\n> > This verifies and depends on \"refs: make rev-parse --quiet actually quiet\".\n> > \n> > t/t1503-rev-parse-verify.sh | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> > \n> > diff --git a/t/t1503-rev-parse-verify.sh b/t/t1503-rev-parse-verify.sh\n> > index 813cc1b..731c21c 100755\n> > --- a/t/t1503-rev-parse-verify.sh\n> > +++ b/t/t1503-rev-parse-verify.sh\n> > @@ -83,6 +83,15 @@ test_expect_success 'fails silently when using -q' '\n> > \ttest -z \"$(cat error)\"\n> > '\n> > \n> > +test_expect_success 'fails silently when using -q with deleted reflogs' '\n> > +\tref=$(git rev-parse HEAD) &&\n> > +\t: >.git/logs/refs/test &&\n> > +\tgit update-ref -m test refs/test \"$ref\" &&\n> \n> I'm just curious, why not simply\n> \n> git branch test\n> ?\n\nMaybe it's a bad reason, but I wanted to replicate the behavior\nthat git stash expects -- it writes to a ref outside of\nrefs/heads/. I thought it'd be good to exercise that same\nmachinery since it will involve different code paths.\n\n> > +\tgit reflog delete --updateref --rewrite refs/test@{0} &&\n> > +\ttest_must_fail git rev-parse --verify --quiet refs/test@{0} 2>error &&\n> \n> Is it a shortcoming of the specification that it doesn't consider\n> whatever might be written to stdout? Is it acceptable that if the\n> git-rev-parse command succeeds, the error message from test_must_fail\n> will be written to the file \"error\" and, therefore, somewhat hidden from\n> the user running the tests?\n\nGood point. The --quiet spec doesn't say anything about stdout,\nbut for this test it probably wouldn't hurt to capture both\nstdout and stderr and assert emptiness.\n\nI can reroll this patch so that 2>error becomes >error 2>&1.\n\n> > +\ttest -z \"$(cat error)\"\n> \n> test(1) comes with an option (-s) to perform such tests and test-lib.sh\n> defines test_must_be_empty which additionally outputs the given file's\n> contents if its not empty.\n\ntest_must_be_empty would be a good fit here. That said, none of\nthe other tests in this file use test_must_be_empty.\n\nIt might be worth doing a follow-up patch that converts all of the\ntests in this file to use test_must_be_empty instead of\ntest -z \"$(cat error)\". I'll reroll.\n\nThanks,\n-- \nDavid\n"},{"id":"249441","messageId":"xmqqy4tk7npg.fsf@gitster.dls.corp.google.com","threadId":"37561","inReplyTo":"20140914185403.GA93515@gmail.com","subject":"Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-15T18:17:31Z","receivedAt":"2014-09-15T18:17:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Good point. The --quiet spec doesn't say anything about stdout,\n\nPlease correct it while at it in the doc ;-)\n\nI think I had to look it up in the documentation and then in code if\n\n git rev-parse --verify --quiet \"$object\"\n\nthe right way to check if the object is a good name without output\nwhen it is, and get diagnosis in an appropriate error message when\nit isn't.\n"},{"id":"249443","messageId":"xmqqppew7nbt.fsf@gitster.dls.corp.google.com","threadId":"37561","inReplyTo":"20140914185403.GA93515@gmail.com","subject":"Re: [PATCH] t1503: test rev-parse --verify --quiet with deleted reflogs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-15T18:25:42Z","receivedAt":"2014-09-15T18:25:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n>> > +test_expect_success 'fails silently when using -q with deleted reflogs' '\n>> > +\tref=$(git rev-parse HEAD) &&\n>> > +\t: >.git/logs/refs/test &&\n>> > +\tgit update-ref -m test refs/test \"$ref\" &&\n>> \n>> I'm just curious, why not simply\n>> \n>> git branch test\n>> ?\n>\n> Maybe it's a bad reason, but I wanted to replicate the behavior\n> that git stash expects -- it writes to a ref outside of\n> refs/heads/. I thought it'd be good to exercise that same\n> machinery since it will involve different code paths.\n\nI think that is a very sensible thing to do. Another reason to\navoid using \"branch\" when you care about what \"update-ref\" does is\nthat \"branch\" does more than what \"update-ref\" does.\n"}]}