Re: [PATCH v2 3/5] t4301: verify that merge-tree fails on missing blob objects

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh
>> @@ -961,4 +961,18 @@ test_expect_success 'error out on missing tree objects' '
>> +test_expect_success 'error out on missing blob objects' '
>> +       seq1=$(test_seq 1 10 | git hash-object -w --stdin) &&
>> +       seq2=$(test_seq 1 11 | git hash-object -w --stdin) &&
>> +       seq3=$(test_seq 0 10 | git hash-object -w --stdin) &&
>
> Is there significance to the ranges passed to test_seq()? Or, can the
> same be achieved by using arbitrary content for each blob?
>
>     blob1=$(echo "one" | git hash-object -w --stdin) &&
>     blob2=$(echo "two" | git hash-object -w --stdin) &&
>     blob3=$(echo "three" | git hash-object -w --stdin) &&
>
>> +       tree1=$(printf "100644 blob %s\tsequence" $seq1 | git mktree) &&
>> +       tree2=$(printf "100644 blob %s\tsequence" $seq2 | git mktree) &&
>> +       tree3=$(printf "100644 blob %s\tsequence" $seq3 | git mktree) &&
>
> I found the lack of terminating "\n" in the `printf` confusing,
> especially since the variable names (seq1, seq2, etc.) and the use of
> `printf` seem to imply, at first glance, that each git-mktree
> invocation is receiving multiple lines as input which, it turns out,
> is not the case. Adding the missing "\n" would help:
>
>    tree1=$(printf "100644 blob %s\tsequence\n" $seq1 | git mktree) &&
>    tree2=$(printf "100644 blob %s\tsequence\n" $seq2 | git mktree) &&
>    tree3=$(printf "100644 blob %s\tsequence\n" $seq3 | git mktree) &&
>
> Interpolating the $seqN variable directly into the string rather than
> using %s would make it even clearer that only a single line is being
> generated as input to git-mktree:
>
>    tree1=$(printf "100644 blob $seq1\tsequence\n" | git mktree) &&
>    tree2=$(printf "100644 blob $seq2\tsequence\n" | git mktree) &&
>    tree3=$(printf "100644 blob $seq3\tsequence\n" | git mktree) &&

All good points.  Thanks for excellent reviews (not just this one
but another one in the series).