Re: [PATCH] p5310: Fix broken && chain in performance test

Stefan Beller [off-list ref] writes:

Signed-off-by: Stefan Beller <redacted>

Thanks.  How did you find this (does the auto &&-chain test apply to
t/perf stuff as well)?

Will queue.

On Fri, Jun 26, 2015 at 3:27 PM, Junio C Hamano [off-list ref] wrote:

Stefan Beller [off-list ref] writes:

quoted

Signed-off-by: Stefan Beller <redacted>

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.

quoted

---
 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

On Fri, Jun 26, 2015 at 03:34:19PM -0700, Stefan Beller wrote:

quoted

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