Re: [PATCH v5 02/11] commit-graph: return with errors during write
- From
Derrick Stolee <stolee@gmail.com>
- Date
- Jul 1, 2019, 12:19 UTC
- Message-ID
- <0e7f4844-69a1-b910-979b-64bd99c240ed@gmail.com>
- In-Reply-To
- <20190629172335.GH21574@szeder.dev>
On 6/29/2019 1:23 PM, SZEDER Gábor wrote:
Show 22 quoted lines
> 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