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

From: Jeff King <peff@peff.net>
Date: Jun 27, 2015, 06:14 UTC
Message-ID: <20150627061444.GB9353@peff.net>
In-Reply-To: <CAGZ79kYCcexkdcF4D4GhUydh=0u0UOZGb0QGbtg6gbKWeO2M_Q@mail.gmail.com>

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

Show 7 quoted lines

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