{"thread":{"id":"39735","subject":"[PATCH] p5310: Fix broken && chain in performance test","startedAt":"2015-06-26T21:27:00Z","lastAt":"2015-06-27T06:14:45Z","messageCount":4,"participants":["Stefan Beller","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"265000","messageId":"1435354020-11675-1-git-send-email-sbeller@google.com","threadId":"39735","inReplyTo":null,"subject":"[PATCH] p5310: Fix broken && chain in performance test","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-26T21:27:00Z","receivedAt":"2015-06-26T21:27:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n t/perf/p5310-pack-bitmaps.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh\nindex f8ed857..de2a224 100755\n--- a/t/perf/p5310-pack-bitmaps.sh\n+++ b/t/perf/p5310-pack-bitmaps.sh\n@@ -39,14 +39,14 @@ test_expect_success 'create partial bitmap state' '\n \n \t# now kill off all of the refs and pretend we had\n \t# just the one tip\n-\trm -rf .git/logs .git/refs/* .git/packed-refs\n-\tgit update-ref HEAD $cutoff\n+\trm -rf .git/logs .git/refs/* .git/packed-refs &&\n+\tgit update-ref HEAD $cutoff &&\n \n \t# and then repack, which will leave us with a nice\n \t# big bitmap pack of the \"old\" history, and all of\n \t# the new history will be loose, as if it had been pushed\n \t# up incrementally and exploded via unpack-objects\n-\tgit repack -Ad\n+\tgit repack -Ad &&\n \n \t# and now restore our original tip, as if the pushes\n \t# had happened\n-- \n2.4.1.345.gab207b6.dirty\n"},{"id":"265010","messageId":"xmqq616artpz.fsf@gitster.dls.corp.google.com","threadId":"39735","inReplyTo":"1435354020-11675-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] p5310: Fix broken && chain in performance test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-06-26T22:27:20Z","receivedAt":"2015-06-26T22:27:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n\nThanks.  How did you find this (does the auto &&-chain test apply to\nt/perf stuff as well)?\n\nWill queue.\n\n> ---\n>  t/perf/p5310-pack-bitmaps.sh | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh\n> index f8ed857..de2a224 100755\n> --- a/t/perf/p5310-pack-bitmaps.sh\n> +++ b/t/perf/p5310-pack-bitmaps.sh\n> @@ -39,14 +39,14 @@ test_expect_success 'create partial bitmap state' '\n>  \n>  \t# now kill off all of the refs and pretend we had\n>  \t# just the one tip\n> -\trm -rf .git/logs .git/refs/* .git/packed-refs\n> -\tgit update-ref HEAD $cutoff\n> +\trm -rf .git/logs .git/refs/* .git/packed-refs &&\n> +\tgit update-ref HEAD $cutoff &&\n>  \n>  \t# and then repack, which will leave us with a nice\n>  \t# big bitmap pack of the \"old\" history, and all of\n>  \t# the new history will be loose, as if it had been pushed\n>  \t# up incrementally and exploded via unpack-objects\n> -\tgit repack -Ad\n> +\tgit repack -Ad &&\n>  \n>  \t# and now restore our original tip, as if the pushes\n>  \t# had happened\n"},{"id":"265011","messageId":"CAGZ79kYCcexkdcF4D4GhUydh=0u0UOZGb0QGbtg6gbKWeO2M_Q@mail.gmail.com","threadId":"39735","inReplyTo":"xmqq616artpz.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] p5310: Fix broken && chain in performance test","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-06-26T22:34:19Z","receivedAt":"2015-06-26T22:34:19Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jun 26, 2015 at 3:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>\n> Thanks.  How did you find this (does the auto &&-chain test apply to\n> t/perf stuff as well)?\n\nApparently the &&-chain tests for it as I got a warning for it while\nbenchmarking some changes in ALLOC_GROW. (which originally\nshould have fixed the coverity false positives, but I was side tracked\nwondering about performance)\n\n>\n> Will queue.\n>\n>> ---\n>>  t/perf/p5310-pack-bitmaps.sh | 6 +++---\n>>  1 file changed, 3 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh\n>> index f8ed857..de2a224 100755\n>> --- a/t/perf/p5310-pack-bitmaps.sh\n>> +++ b/t/perf/p5310-pack-bitmaps.sh\n>> @@ -39,14 +39,14 @@ test_expect_success 'create partial bitmap state' '\n>>\n>>       # now kill off all of the refs and pretend we had\n>>       # just the one tip\n>> -     rm -rf .git/logs .git/refs/* .git/packed-refs\n>> -     git update-ref HEAD $cutoff\n>> +     rm -rf .git/logs .git/refs/* .git/packed-refs &&\n>> +     git update-ref HEAD $cutoff &&\n>>\n>>       # and then repack, which will leave us with a nice\n>>       # big bitmap pack of the \"old\" history, and all of\n>>       # the new history will be loose, as if it had been pushed\n>>       # up incrementally and exploded via unpack-objects\n>> -     git repack -Ad\n>> +     git repack -Ad &&\n>>\n>>       # and now restore our original tip, as if the pushes\n>>       # had happened\n"},{"id":"265025","messageId":"20150627061444.GB9353@peff.net","threadId":"39735","inReplyTo":"CAGZ79kYCcexkdcF4D4GhUydh=0u0UOZGb0QGbtg6gbKWeO2M_Q@mail.gmail.com","subject":"Re: [PATCH] p5310: Fix broken && chain in performance test","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-06-27T06:14:45Z","receivedAt":"2015-06-27T06:14:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 26, 2015 at 03:34:19PM -0700, Stefan Beller wrote:\n\n> > Thanks.  How did you find this (does the auto &&-chain test apply to\n> > t/perf stuff as well)?\n> \n> Apparently the &&-chain tests for it as I got a warning for it while\n> benchmarking some changes in ALLOC_GROW. (which originally\n> should have fixed the coverity false positives, but I was side tracked\n> wondering about performance)\n\nThat makes sense; we use test_expect_success here, so I think it is good\nfor it to check that we are actually creating a sane exit status.\n\nIt looks like we do not extend the same protection to test_perf (it uses\ntest_eval_, bnot test_run_). That is probably OK, as those tests are\nmore about measuring the time than about correctness. OTOH, we do notice\nwhen a test_perf reports failure, so perhaps it would make sense to\nextend &&-chaining checks there.\n\n-Peff\n"}]}