Re: [PATCH v5 02/11] commit-graph: return with errors during write

[Derrick Stolee <stolee@xxxxxxxxx>]

On 6/29/2019 1:23 PM, SZEDER Gábor wrote:
> On Wed, Jun 12, 2019 at 06:29:37AM -0700, Derrick Stolee via GitGitGadget wrote:
>> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
>> index e80c1cac02..3b6fd0d728 100755
>> --- a/t/t5318-commit-graph.sh
>> +++ b/t/t5318-commit-graph.sh
>> @@ -23,6 +23,14 @@ test_expect_success 'write graph with no packs' '
>>  	test_path_is_file info/commit-graph
>>  '
>>  
>> +test_expect_success 'close with correct error on bad input' '
>> +	cd "$TRASH_DIRECTORY/full" &&
>> +	echo doesnotexist >in &&
>> +	{ git commit-graph write --stdin-packs <in 2>stderr; ret=$?; } &&
>> +	test "$ret" = 1 &&
> 
> This could be: 
> 
>   test_expect_code 1 git commit-graph write --stdin-packs <in 2>stderr
> 
> 
>> +	test_i18ngrep "error adding pack" stderr
>> +'

Thanks!, you are right! test_expect_code is what I should have used here
instead of finding the "ret=$?" trick in t0005-signals.sh, which needs to
do more interesting logic on the return code.

Here is your suggestion as a diff. Junio: could you squash this in, or
should I submit a full patch?

diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 22cb9d66430..4391007f4c1 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -26,8 +26,7 @@ test_expect_success 'write graph with no packs' '
 test_expect_success 'close with correct error on bad input' '
        cd "$TRASH_DIRECTORY/full" &&
        echo doesnotexist >in &&
-       { git commit-graph write --stdin-packs <in 2>stderr; ret=$?; } &&
-       test "$ret" = 1 &&
+       test_expect_code 1 git commit-graph write --stdin-packs <in 2>stderr &&
        test_i18ngrep "error adding pack" stderr
 '

I took inventory of when we are using "=$?" in the test scripts and saw
this was the only one that could easily be removed. Every other place is
doing something that can't be replaced by test_expect_code.

Thanks,
-Stolee