# [PATCH] p5310: Fix broken && chain in performance test 4 messages from 2015-06-26 to 2015-06-27. Participants: Stefan Beller, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/39735 ## Stefan Beller, 2015-06-26 21:27 Subject: [PATCH] p5310: Fix broken && chain in performance test Message-ID: <1435354020-11675-1-git-send-email-sbeller@google.com> URL: https://gitlist.dev/e/1435354020-11675-1-git-send-email-sbeller%40google.com ``` Signed-off-by: Stefan Beller --- t/perf/p5310-pack-bitmaps.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh index f8ed857..de2a224 100755 --- a/t/perf/p5310-pack-bitmaps.sh +++ b/t/perf/p5310-pack-bitmaps.sh @@ -39,14 +39,14 @@ test_expect_success 'create partial bitmap state' ' # now kill off all of the refs and pretend we had # just the one tip - rm -rf .git/logs .git/refs/* .git/packed-refs - git update-ref HEAD $cutoff + rm -rf .git/logs .git/refs/* .git/packed-refs && + git update-ref HEAD $cutoff && # and then repack, which will leave us with a nice # big bitmap pack of the "old" history, and all of # the new history will be loose, as if it had been pushed # up incrementally and exploded via unpack-objects - git repack -Ad + git repack -Ad && # and now restore our original tip, as if the pushes # had happened -- 2.4.1.345.gab207b6.dirty ``` ## Junio C Hamano, 2015-06-26 22:27 Subject: Re: [PATCH] p5310: Fix broken && chain in performance test Message-ID: URL: https://gitlist.dev/e/xmqq616artpz.fsf%40gitster.dls.corp.google.com In-Reply-To: <1435354020-11675-1-git-send-email-sbeller@google.com> ``` Stefan Beller writes: > Signed-off-by: Stefan Beller Thanks. How did you find this (does the auto &&-chain test apply to t/perf stuff as well)? Will queue. > --- > t/perf/p5310-pack-bitmaps.sh | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh > index f8ed857..de2a224 100755 > --- a/t/perf/p5310-pack-bitmaps.sh > +++ b/t/perf/p5310-pack-bitmaps.sh > @@ -39,14 +39,14 @@ test_expect_success 'create partial bitmap state' ' > > # now kill off all of the refs and pretend we had > # just the one tip > - rm -rf .git/logs .git/refs/* .git/packed-refs > - git update-ref HEAD $cutoff > + rm -rf .git/logs .git/refs/* .git/packed-refs && > + git update-ref HEAD $cutoff && > > # and then repack, which will leave us with a nice > # big bitmap pack of the "old" history, and all of > # the new history will be loose, as if it had been pushed > # up incrementally and exploded via unpack-objects > - git repack -Ad > + git repack -Ad && > > # and now restore our original tip, as if the pushes > # had happened ``` ## Stefan Beller, 2015-06-26 22:34 Subject: Re: [PATCH] p5310: Fix broken && chain in performance test Message-ID: URL: https://gitlist.dev/e/CAGZ79kYCcexkdcF4D4GhUydh%3D0u0UOZGb0QGbtg6gbKWeO2M_Q%40mail.gmail.com In-Reply-To: ``` On Fri, Jun 26, 2015 at 3:27 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> Signed-off-by: Stefan Beller > > Thanks. How did you find this (does the auto &&-chain test apply to > t/perf stuff as well)? Apparently the &&-chain tests for it as I got a warning for it while benchmarking some changes in ALLOC_GROW. (which originally should have fixed the coverity false positives, but I was side tracked wondering about performance) > > Will queue. > >> --- >> t/perf/p5310-pack-bitmaps.sh | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/t/perf/p5310-pack-bitmaps.sh b/t/perf/p5310-pack-bitmaps.sh >> index f8ed857..de2a224 100755 >> --- a/t/perf/p5310-pack-bitmaps.sh >> +++ b/t/perf/p5310-pack-bitmaps.sh >> @@ -39,14 +39,14 @@ test_expect_success 'create partial bitmap state' ' >> >> # now kill off all of the refs and pretend we had >> # just the one tip >> - rm -rf .git/logs .git/refs/* .git/packed-refs >> - git update-ref HEAD $cutoff >> + rm -rf .git/logs .git/refs/* .git/packed-refs && >> + git update-ref HEAD $cutoff && >> >> # and then repack, which will leave us with a nice >> # big bitmap pack of the "old" history, and all of >> # the new history will be loose, as if it had been pushed >> # up incrementally and exploded via unpack-objects >> - git repack -Ad >> + git repack -Ad && >> >> # and now restore our original tip, as if the pushes >> # had happened ``` ## Jeff King, 2015-06-27 06:14 Subject: Re: [PATCH] p5310: Fix broken && chain in performance test Message-ID: <20150627061444.GB9353@peff.net> URL: https://gitlist.dev/e/20150627061444.GB9353%40peff.net In-Reply-To: ``` On Fri, Jun 26, 2015 at 03:34:19PM -0700, Stefan Beller wrote: > > Thanks. How did you find this (does the auto &&-chain test apply to > > t/perf stuff as well)? > > Apparently the &&-chain tests for it as I got a warning for it while > benchmarking some changes in ALLOC_GROW. (which originally > should have fixed the coverity false positives, but I was side tracked > wondering about performance) That makes sense; we use test_expect_success here, so I think it is good for it to check that we are actually creating a sane exit status. It looks like we do not extend the same protection to test_perf (it uses test_eval_, bnot test_run_). That is probably OK, as those tests are more about measuring the time than about correctness. OTOH, we do notice when a test_perf reports failure, so perhaps it would make sense to extend &&-chaining checks there. -Peff ```