diff mbox series

[RFC,v2,3/4] t7006-pager.sh: more lenient trace checking

Message ID 20210423001539.4059524-4-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series share a config between submodule and superproject | expand

Commit Message

Emily Shaffer April 23, 2021, 12:15 a.m. UTC 
A number of tests in t7006-pager.sh are, as a side effect, checking that
'git log' does not invoke any child processes besides the pager. There
is no reason for that guarantee, and it is not explicitly the purpose of
these tests, so let's make the checking more intelligent and flexible.

child_start and child_exit events share a child ID - using that child
ID, we can try to disambiguate which child_exit belongs to which
child_start and limit our validation to only the pager's child process.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 t/t7006-pager.sh | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Comments

Phillip Wood April 23, 2021, 9:54 a.m. UTC | #1 
Hi Emily

On 23/04/2021 01:15, Emily Shaffer wrote:
> A number of tests in t7006-pager.sh are, as a side effect, checking that
> 'git log' does not invoke any child processes besides the pager. There
> is no reason for that guarantee, and it is not explicitly the purpose of
> these tests, so let's make the checking more intelligent and flexible.
> 
> child_start and child_exit events share a child ID - using that child
> ID, we can try to disambiguate which child_exit belongs to which
> child_start and limit our validation to only the pager's child process.

I've not looked at this test file properly but if we want to check that 
the pager is invoked can we use a script that writes a file when it has 
been run as the pager and just cleanup that file at the end of each test 
case?

> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>   t/t7006-pager.sh | 24 ++++++++++++++++++------
>   1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> index 0e7cf75435..ac2d91d56b 100755
> --- a/t/t7006-pager.sh
> +++ b/t/t7006-pager.sh
> @@ -676,7 +676,9 @@ test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
>   		test_terminal git log
>   	fi &&
>   
> -	grep child_exit trace.normal >child-exits &&
> +	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
> +			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&

If you want to save a process you could use sed to do the job of the 
grep command. I think this should do it

sed -n -e "/child_exit/ {" -e "s/child_start\[\([0-9]\+\)\].*/\1/p" -e "}"

> +	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&

Why is $PAGER_CHILD_ID unquoted?

Best Wishes

Phillip

>   	test_line_count = 1 child-exits &&
>   	grep " code:0 " child-exits &&
>   	test_path_is_file pager-used
> @@ -697,7 +699,9 @@ test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '
>   		test_terminal git log
>   	fi &&
>   
> -	grep child_exit trace.normal >child-exits &&
> +	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
> +			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
> +	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>   	test_line_count = 1 child-exits &&
>   	grep " code:1 " child-exits &&
>   	test_path_is_file pager-used
> @@ -718,7 +722,9 @@ test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '
>   		test_terminal git log
>   	fi &&
>   
> -	grep child_exit trace.normal >child-exits &&
> +	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
> +			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
> +	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>   	test_line_count = 1 child-exits &&
>   	grep " code:1 " child-exits &&
>   	test_path_is_file pager-used
> @@ -739,7 +745,9 @@ test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '
>   		test_terminal git log
>   	fi &&
>   
> -	grep child_exit trace.normal >child-exits &&
> +	PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \
> +			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
> +	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>   	test_line_count = 1 child-exits &&
>   	grep " code:127 " child-exits &&
>   	test_path_is_file pager-used
> @@ -760,7 +768,9 @@ test_expect_success TTY 'git attempts to page to nonexisting pager command, gets
>   		test_terminal git log
>   	fi &&
>   
> -	grep child_exit trace.normal >child-exits &&
> +	PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \
> +			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
> +	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>   	test_line_count = 1 child-exits &&
>   	grep " code:-1 " child-exits
>   '
> @@ -780,7 +790,9 @@ test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '
>   		test_terminal git log
>   	fi &&
>   
> -	grep child_exit trace.normal >child-exits &&
> +	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
> +			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
> +	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>   	test_line_count = 1 child-exits &&
>   	grep " code:143 " child-exits &&
>   	test_path_is_file pager-used
>
Phillip Wood April 23, 2021, 12:45 p.m. UTC | #2 
Hi Emily

On 23/04/2021 10:54, Phillip Wood wrote:
> Hi Emily
> [...]
>> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
>> index 0e7cf75435..ac2d91d56b 100755
>> --- a/t/t7006-pager.sh
>> +++ b/t/t7006-pager.sh
>> @@ -676,7 +676,9 @@ test_expect_success TTY 'git returns SIGPIPE on 
>> early pager exit' '
>>           test_terminal git log
>>       fi &&
>> -    grep child_exit trace.normal >child-exits &&
>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \
>> +             sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
> 
> If you want to save a process you could use sed to do the job of the 
> grep command. I think this should do it
> 
> sed -n -e "/child_exit/ {" -e "s/child_start\[\([0-9]\+\)\].*/\1/p" -e "}"

I must have been half asleep when I wrote that, there is no need for the 
braces and the initial match is wrong it should be

sed -n "/pager-used/s/child_start\[\([0-9]\+\)\].*/\1/p"


Best Wishes

Phillip

>> +    grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
> 
> Why is $PAGER_CHILD_ID unquoted?
> 
> Best Wishes
> 
> Phillip
> 
>>       test_line_count = 1 child-exits &&
>>       grep " code:0 " child-exits &&
>>       test_path_is_file pager-used
>> @@ -697,7 +699,9 @@ test_expect_success TTY 'git returns SIGPIPE on 
>> early pager non-zero exit' '
>>           test_terminal git log
>>       fi &&
>> -    grep child_exit trace.normal >child-exits &&
>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \
>> +             sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
>> +    grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>>       test_line_count = 1 child-exits &&
>>       grep " code:1 " child-exits &&
>>       test_path_is_file pager-used
>> @@ -718,7 +722,9 @@ test_expect_success TTY 'git discards pager 
>> non-zero exit without SIGPIPE' '
>>           test_terminal git log
>>       fi &&
>> -    grep child_exit trace.normal >child-exits &&
>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \
>> +             sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
>> +    grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>>       test_line_count = 1 child-exits &&
>>       grep " code:1 " child-exits &&
>>       test_path_is_file pager-used
>> @@ -739,7 +745,9 @@ test_expect_success TTY 'git discards nonexisting 
>> pager without SIGPIPE' '
>>           test_terminal git log
>>       fi &&
>> -    grep child_exit trace.normal >child-exits &&
>> +    PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \
>> +             sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
>> +    grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>>       test_line_count = 1 child-exits &&
>>       grep " code:127 " child-exits &&
>>       test_path_is_file pager-used
>> @@ -760,7 +768,9 @@ test_expect_success TTY 'git attempts to page to 
>> nonexisting pager command, gets
>>           test_terminal git log
>>       fi &&
>> -    grep child_exit trace.normal >child-exits &&
>> +    PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \
>> +             sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
>> +    grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>>       test_line_count = 1 child-exits &&
>>       grep " code:-1 " child-exits
>>   '
>> @@ -780,7 +790,9 @@ test_expect_success TTY 'git returns SIGPIPE on 
>> propagated signals from pager' '
>>           test_terminal git log
>>       fi &&
>> -    grep child_exit trace.normal >child-exits &&
>> +    PAGER_CHILD_ID=$(grep pager-used trace.normal | \
>> +             sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
>> +    grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
>>       test_line_count = 1 child-exits &&
>>       grep " code:143 " child-exits &&
>>       test_path_is_file pager-used
>>
>
diff mbox series

Patch

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 0e7cf75435..ac2d91d56b 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -676,7 +676,9 @@  test_expect_success TTY 'git returns SIGPIPE on early pager exit' '
 		test_terminal git log
 	fi &&
 
-	grep child_exit trace.normal >child-exits &&
+	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
+			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
+	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
 	test_line_count = 1 child-exits &&
 	grep " code:0 " child-exits &&
 	test_path_is_file pager-used
@@ -697,7 +699,9 @@  test_expect_success TTY 'git returns SIGPIPE on early pager non-zero exit' '
 		test_terminal git log
 	fi &&
 
-	grep child_exit trace.normal >child-exits &&
+	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
+			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
+	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
 	test_line_count = 1 child-exits &&
 	grep " code:1 " child-exits &&
 	test_path_is_file pager-used
@@ -718,7 +722,9 @@  test_expect_success TTY 'git discards pager non-zero exit without SIGPIPE' '
 		test_terminal git log
 	fi &&
 
-	grep child_exit trace.normal >child-exits &&
+	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
+			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
+	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
 	test_line_count = 1 child-exits &&
 	grep " code:1 " child-exits &&
 	test_path_is_file pager-used
@@ -739,7 +745,9 @@  test_expect_success TTY 'git discards nonexisting pager without SIGPIPE' '
 		test_terminal git log
 	fi &&
 
-	grep child_exit trace.normal >child-exits &&
+	PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \
+			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
+	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
 	test_line_count = 1 child-exits &&
 	grep " code:127 " child-exits &&
 	test_path_is_file pager-used
@@ -760,7 +768,9 @@  test_expect_success TTY 'git attempts to page to nonexisting pager command, gets
 		test_terminal git log
 	fi &&
 
-	grep child_exit trace.normal >child-exits &&
+	PAGER_CHILD_ID=$(grep does-not-exist trace.normal | \
+			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
+	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
 	test_line_count = 1 child-exits &&
 	grep " code:-1 " child-exits
 '
@@ -780,7 +790,9 @@  test_expect_success TTY 'git returns SIGPIPE on propagated signals from pager' '
 		test_terminal git log
 	fi &&
 
-	grep child_exit trace.normal >child-exits &&
+	PAGER_CHILD_ID=$(grep pager-used trace.normal | \
+			 sed -n "s/child_start\[\([0-9]\+\)\].*/\1/p") &&
+	grep -F "child_exit["$PAGER_CHILD_ID"]" trace.normal >child-exits &&
 	test_line_count = 1 child-exits &&
 	grep " code:143 " child-exits &&
 	test_path_is_file pager-used