Thread (10 messages) 10 messages, 2 authors, 2017-03-03

Re: [PATCH] t/perf: export variable used in other blocks

flat view

From: Jeff King <hidden>
Date: 2017-03-03 06:53:40

On Thu, Mar 02, 2017 at 11:50:41AM -0800, Jonathan Tan wrote:
In p0001, a variable was created in a test_expect_success block to be
used in later test_perf blocks, but was not exported. This caused the
variable to not appear in those blocks (this can be verified by writing
'test -n "$commit"' in those blocks), resulting in a slightly different
invocation than what was intended. Export that variable.
Thanks, this is obviously the right thing to do, and the mistake is mine
from ea97002fc (t/perf: time rev-list with UNINTERESTING commits,
2014-01-20). This is not the first time I've been confused by missing
variables in t/perf scripts, since it behaves differently than the
normal test suite. I wonder if we should turn on "set -a" during t/perf
setup snippets. That's a bit of a blunt tool, but I suspect it would
just be easier to work with.

I was curious that the tests added by ea97002fc showed something useful
even with the bug you're fixing here. But it's because the actual slice
of history we meant to traverse isn't important. It's intentionally
tiny to show off the time spent dealing with the UNINTERESTING commits.
So in effect we were traversing no commits instead of a tiny set, but
the timing results were the same.

I repeated the tests over fbd4a703 given in the commit message of
ee9a7002fc and confirmed that it behaves the same with your fixed
version of the test. I did have to tweak a few other things to get the
test to run against such an old version of git, though. I'll follow-up
with a patch.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help