diff mbox series

[2/3] t6050: redirect expected error output to /dev/null

Message ID 20190328171722.9753-2-chriscool@tuxfamily.org (mailing list archive)
State New, archived
Headers show
Series [1/3] t6050: use test_line_count instead of wc -l | expand

Commit Message

Christian Couder March 28, 2019, 5:17 p.m. UTC
Otherwise the error from `git rev-parse` is uselessly
polluting the debug output.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t6050-replace.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eric Sunshine March 28, 2019, 6:41 p.m. UTC | #1
On Thu, Mar 28, 2019 at 1:17 PM Christian Couder
<christian.couder@gmail.com> wrote:
> Otherwise the error from `git rev-parse` is uselessly
> polluting the debug output.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> @@ -40,7 +40,7 @@ commit_peeling_shows_parents ()
> -    test_must_fail git rev-parse --verify $_commit^$_parent_number
> +    test_must_fail git rev-parse --verify $_commit^$_parent_number 2>/dev/null

When I'm debugging test failures, I find it very helpful to see that a
command which was expected to fail did indeed fail with the expected
error message, so I don't consider such a message as "uselessly
polluting the debug output". Consequently, I'm rather negative on this
change.
Ævar Arnfjörð Bjarmason March 28, 2019, 8:22 p.m. UTC | #2
On Thu, Mar 28 2019, Eric Sunshine wrote:

> On Thu, Mar 28, 2019 at 1:17 PM Christian Couder
> <christian.couder@gmail.com> wrote:
>> Otherwise the error from `git rev-parse` is uselessly
>> polluting the debug output.
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
>> @@ -40,7 +40,7 @@ commit_peeling_shows_parents ()
>> -    test_must_fail git rev-parse --verify $_commit^$_parent_number
>> +    test_must_fail git rev-parse --verify $_commit^$_parent_number 2>/dev/null
>
> When I'm debugging test failures, I find it very helpful to see that a
> command which was expected to fail did indeed fail with the expected
> error message, so I don't consider such a message as "uselessly
> polluting the debug output". Consequently, I'm rather negative on this
> change.

I believe this instead should make both of you happy:

    diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
    index d638119750..2e3009f3cb 100755
    --- a/t/t6050-replace.sh
    +++ b/t/t6050-replace.sh
    @@ -40,7 +40,8 @@ commit_peeling_shows_parents ()
            test "$_found" = "$_parent" || return 1
            _parent_number=$(( $_parent_number + 1 ))
         done &&
    -    test_must_fail git rev-parse --verify $_commit^$_parent_number
    +    test_must_fail git rev-parse --verify $_commit^$_parent_number 2>err &&
    +    test_i18ngrep "Needed a single revision" err
     }

     commit_has_parents ()

Note that the file has existing 4-space-instead-of-tabs issues.
diff mbox series

Patch

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 41b177936e..5cb8281bab 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -40,7 +40,7 @@  commit_peeling_shows_parents ()
 	test "$_found" = "$_parent" || return 1
 	_parent_number=$(( $_parent_number + 1 ))
     done &&
-    test_must_fail git rev-parse --verify $_commit^$_parent_number
+    test_must_fail git rev-parse --verify $_commit^$_parent_number 2>/dev/null
 }
 
 commit_has_parents ()