Re: [PATCH v2 7/8] for-each-ref: add ahead-behind format atom

>> +test_description='Commit walk performance tests'
>> +. ./perf-lib.sh
>> +
>> +test_perf_large_repo
>> +
>> +test_expect_success 'setup' '
>> +	git for-each-ref --format="%(refname)" "refs/heads/*" "refs/tags/*" >allrefs &&
>> +	sort -r allrefs | head -n 50 >refs &&
> 
> Some of the point of test_perf_large_repo is being able to point the
> test to an arbitrary sized repo, why "head -n 50" here, instead of just
> doing that filtering when preparing the test repo?

>> +test_expect_success 'ahead-behind requires an argument' '
>> +	test_must_fail git for-each-ref \
>> +		--format="%(ahead-behind)" 2>err &&
>> +	grep "expected format: %(ahead-behind:<ref>)" err
>> +'
>> +
>> +test_expect_success 'missing ahead-behind base' '
>> +	test_must_fail git for-each-ref \
>> +		--format="%(ahead-behind:refs/heads/missing)" 2>err &&
>> +	grep "failed to find '\''refs/heads/missing'\''" err
>> +'
>> +
> 
> Is this grep instead of test_cmp for brevity, or because we'll catch
> this late and spew out other output as well?
> 
> I'd think it would be worth testing that we only emit an error. Even if
> you don't want a full test_cmp we could check the line count too to
> assert that...

>> +# Run this before doing any signing, so the test has the same results
>> +# regardless of the GPG prereq.
>> +test_expect_success 'git tag --format with ahead-behind' '
>> +	test_when_finished git reset --hard tag-one-line &&
>> +	git commit --allow-empty -m "left" &&
>> +	git tag -a -m left tag-left &&
>> +	git reset --hard HEAD~1 &&
>> +	git commit --allow-empty -m "right" &&
>> +	git tag -a -m left tag-right &&
> 
> Do we really need this --allow-empty insted of just using "test_commit"?
> I.e. is being TREESAME here important?

>> [...] Also, the
>> test in t7004 is carefully located to avoid being dependent on the GPG
>> prereq. It also avoids using the test_commit helper, as that will add
>> ticks to the time and disrupt the expected timestampes in later tag
>> tests.