git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [GSoC][PATCH 0/1] Microproject Submission
@ 2020-03-22 12:46 Shanthanu
  2020-03-22 12:46 ` [GSoC][PATCH 1/1] t9116: avoid using pipes Shanthanu
  2020-03-24  9:35 ` [PATCH v2] " Shanthanu
  0 siblings, 2 replies; 7+ messages in thread
From: Shanthanu @ 2020-03-22 12:46 UTC (permalink / raw)
  To: git; +Cc: Shanthanu

Hello! I am submitting this patch as part of my GSoC application.

Shanthanu (1):
  t9116: avoid using pipes

 t/t9116-git-svn-log.sh | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

-- 
2.26.0.rc2.28.g7fcb965970


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [GSoC][PATCH 1/1] t9116: avoid using pipes
  2020-03-22 12:46 [GSoC][PATCH 0/1] Microproject Submission Shanthanu
@ 2020-03-22 12:46 ` Shanthanu
  2020-03-22 17:36   ` Torsten Bögershausen
  2020-03-24  9:35 ` [PATCH v2] " Shanthanu
  1 sibling, 1 reply; 7+ messages in thread
From: Shanthanu @ 2020-03-22 12:46 UTC (permalink / raw)
  To: git; +Cc: Shanthanu

Commit de26f02db1 (t9001, t9116: avoid pipes, 2020-02-14) noticed that
when grepping through the output of a command, there is always a
chance that something could go wrong.

So, redirect the output into a file and grep that file. Thus the log
can be inspected easily if the grep fails.

Signed-off-by: Shanthanu <shanthanu.s.rai9@gmail.com>
---

In test 'test ascending revision range with --show-commit (sha1)',
'out1' variable is used since 'out' variable was already in use. 

 t/t9116-git-svn-log.sh | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
index 0a9f1ef366..d82aa0fab9 100755
--- a/t/t9116-git-svn-log.sh
+++ b/t/t9116-git-svn-log.sh
@@ -61,12 +61,14 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
 
 test_expect_success 'test ascending revision range' "
 	git reset --hard origin/trunk &&
-	git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
+	git svn log -r 1:4 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
 	"
 
 test_expect_success 'test ascending revision range with --show-commit' "
 	git reset --hard origin/trunk &&
-	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
+	git svn log --show-commit -r 1:4 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
 	"
 
 test_expect_success 'test ascending revision range with --show-commit (sha1)' "
@@ -74,7 +76,8 @@ test_expect_success 'test ascending revision range with --show-commit (sha1)' "
 	git svn find-rev r2 >>expected-range-r1-r2-r4-sha1 &&
 	git svn find-rev r4 >>expected-range-r1-r2-r4-sha1 &&
 	git reset --hard origin/trunk &&
-	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f2 >out &&
+	git svn log --show-commit -r 1:4 >out1 &&
+	grep '^r[0-9]' out1 | cut -d'|' -f2 >out &&
 	git rev-parse \$(cat out) >actual &&
 	test_cmp expected-range-r1-r2-r4-sha1 actual
 	"
@@ -83,45 +86,52 @@ printf 'r4 \nr2 \nr1 \n' > expected-range-r4-r2-r1
 
 test_expect_success 'test descending revision range' "
 	git reset --hard origin/trunk &&
-	git svn log -r 4:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
+	git svn log -r 4:1 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
 	"
 
 printf 'r1 \nr2 \n' > expected-range-r1-r2
 
 test_expect_success 'test ascending revision range with unreachable revision' "
 	git reset --hard origin/trunk &&
-	git svn log -r 1:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
+	git svn log -r 1:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
 	"
 
 printf 'r2 \nr1 \n' > expected-range-r2-r1
 
 test_expect_success 'test descending revision range with unreachable revision' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
+	git svn log -r 3:1 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
 	"
 
 printf 'r2 \n' > expected-range-r2
 
 test_expect_success 'test ascending revision range with unreachable upper boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 2:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
+	git svn log -r 2:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2 -
 	"
 
 test_expect_success 'test descending revision range with unreachable upper boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:2 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
+	git svn log -r 3:2 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2 -
 	"
 
 printf 'r4 \n' > expected-range-r4
 
 test_expect_success 'test ascending revision range with unreachable lower boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 3:4 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 test_expect_success 'test descending revision range with unreachable lower boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 4:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 4:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 printf -- '------------------------------------------------------------------------\n' > expected-separator
@@ -138,12 +148,14 @@ test_expect_success 'test descending revision range with unreachable boundary re
 
 test_expect_success 'test ascending revision range with unreachable boundary revisions and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:5 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 3:5 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 test_expect_success 'test descending revision range with unreachable boundary revisions and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 5:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 5:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
 	"
 
 test_done
-- 
2.26.0.rc2.28.g7fcb965970


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [GSoC][PATCH 1/1] t9116: avoid using pipes
  2020-03-22 12:46 ` [GSoC][PATCH 1/1] t9116: avoid using pipes Shanthanu
@ 2020-03-22 17:36   ` Torsten Bögershausen
  2020-03-23  7:04     ` Shanthanu
  0 siblings, 1 reply; 7+ messages in thread
From: Torsten Bögershausen @ 2020-03-22 17:36 UTC (permalink / raw)
  To: Shanthanu; +Cc: git

Thanks for contributing to Git.

On Sun, Mar 22, 2020 at 06:16:19PM +0530, Shanthanu wrote:
> Commit de26f02db1 (t9001, t9116: avoid pipes, 2020-02-14) noticed that
> when grepping through the output of a command, there is always a
> chance that something could go wrong.

That "something could go wrong" may deserve little bit more explanation
in the commit message.
One thing when using pipes is that we may loose return codes of programs
being part of the pipe.
The other thing is that it is hard to debug, if the test case fails.
Having the "out" file left on disk allows manual inspection, and tracking
down why it failed.
Having said this, we can make the test case even easier to debug, and consistant
with other test cases, please see below.

>
> So, redirect the output into a file and grep that file. Thus the log
> can be inspected easily if the grep fails.
>
> Signed-off-by: Shanthanu <shanthanu.s.rai9@gmail.com>
> ---
>
> In test 'test ascending revision range with --show-commit (sha1)',
> 'out1' variable is used since 'out' variable was already in use.
>
>  t/t9116-git-svn-log.sh | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
> index 0a9f1ef366..d82aa0fab9 100755
> --- a/t/t9116-git-svn-log.sh
> +++ b/t/t9116-git-svn-log.sh
> @@ -61,12 +61,14 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
>
>  test_expect_success 'test ascending revision range' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
> +	git svn log -r 1:4 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -

That could be written as
	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
	test_cmp expected-range-r1-r2-r4 actual

Or something in that style  (and similar below).
What do you think ?

>  	"
>
>  test_expect_success 'test ascending revision range with --show-commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
> +	git svn log --show-commit -r 1:4 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
>  	"
>
>  test_expect_success 'test ascending revision range with --show-commit (sha1)' "
> @@ -74,7 +76,8 @@ test_expect_success 'test ascending revision range with --show-commit (sha1)' "
>  	git svn find-rev r2 >>expected-range-r1-r2-r4-sha1 &&
>  	git svn find-rev r4 >>expected-range-r1-r2-r4-sha1 &&
>  	git reset --hard origin/trunk &&
> -	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f2 >out &&
> +	git svn log --show-commit -r 1:4 >out1 &&
> +	grep '^r[0-9]' out1 | cut -d'|' -f2 >out &&
>  	git rev-parse \$(cat out) >actual &&
>  	test_cmp expected-range-r1-r2-r4-sha1 actual
>  	"
> @@ -83,45 +86,52 @@ printf 'r4 \nr2 \nr1 \n' > expected-range-r4-r2-r1
>
>  test_expect_success 'test descending revision range' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 4:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
> +	git svn log -r 4:1 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
>  	"
>
>  printf 'r1 \nr2 \n' > expected-range-r1-r2
>
>  test_expect_success 'test ascending revision range with unreachable revision' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 1:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
> +	git svn log -r 1:3 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
>  	"
>
>  printf 'r2 \nr1 \n' > expected-range-r2-r1
>
>  test_expect_success 'test descending revision range with unreachable revision' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 3:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
> +	git svn log -r 3:1 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
>  	"
>
>  printf 'r2 \n' > expected-range-r2
>
>  test_expect_success 'test ascending revision range with unreachable upper boundary revision and 1 commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 2:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
> +	git svn log -r 2:3 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2 -
>  	"
>
>  test_expect_success 'test descending revision range with unreachable upper boundary revision and 1 commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 3:2 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
> +	git svn log -r 3:2 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2 -
>  	"
>
>  printf 'r4 \n' > expected-range-r4
>
>  test_expect_success 'test ascending revision range with unreachable lower boundary revision and 1 commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 3:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
> +	git svn log -r 3:4 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>  	"
>
>  test_expect_success 'test descending revision range with unreachable lower boundary revision and 1 commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 4:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
> +	git svn log -r 4:3 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>  	"
>
>  printf -- '------------------------------------------------------------------------\n' > expected-separator
> @@ -138,12 +148,14 @@ test_expect_success 'test descending revision range with unreachable boundary re
>
>  test_expect_success 'test ascending revision range with unreachable boundary revisions and 1 commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 3:5 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
> +	git svn log -r 3:5 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>  	"
>
>  test_expect_success 'test descending revision range with unreachable boundary revisions and 1 commit' "
>  	git reset --hard origin/trunk &&
> -	git svn log -r 5:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
> +	git svn log -r 5:3 >out &&
> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>  	"
>
>  test_done
> --
> 2.26.0.rc2.28.g7fcb965970
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [GSoC][PATCH 1/1] t9116: avoid using pipes
  2020-03-22 17:36   ` Torsten Bögershausen
@ 2020-03-23  7:04     ` Shanthanu
  0 siblings, 0 replies; 7+ messages in thread
From: Shanthanu @ 2020-03-23  7:04 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git

Thanks for the review!

On 22/03/20 11:06 pm, Torsten Bögershausen wrote:
>> Commit de26f02db1 (t9001, t9116: avoid pipes, 2020-02-14) noticed that
>> when grepping through the output of a command, there is always a
>> chance that something could go wrong.
> That "something could go wrong" may deserve little bit more explanation
> in the commit message.
> One thing when using pipes is that we may loose return codes of programs
> being part of the pipe.
> The other thing is that it is hard to debug, if the test case fails.
> Having the "out" file left on disk allows manual inspection, and tracking
> down why it failed.

Yeah makes sense. I will update the commit message by adding the above
details.

> Having said this, we can make the test case even easier to debug, and consistant
> with other test cases, please see below.
>
>> So, redirect the output into a file and grep that file. Thus the log
>> can be inspected easily if the grep fails.
>>
>> Signed-off-by: Shanthanu <shanthanu.s.rai9@gmail.com>
>> ---
>>
>> In test 'test ascending revision range with --show-commit (sha1)',
>> 'out1' variable is used since 'out' variable was already in use.
>>
>>  t/t9116-git-svn-log.sh | 36 ++++++++++++++++++++++++------------
>>  1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
>> index 0a9f1ef366..d82aa0fab9 100755
>> --- a/t/t9116-git-svn-log.sh
>> +++ b/t/t9116-git-svn-log.sh
>> @@ -61,12 +61,14 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
>>
>>  test_expect_success 'test ascending revision range' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
>> +	git svn log -r 1:4 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
> That could be written as
> 	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
> 	test_cmp expected-range-r1-r2-r4 actual
>
> Or something in that style  (and similar below).
> What do you think ?
>
>>  	"
>>
>>  test_expect_success 'test ascending revision range with --show-commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
>> +	git svn log --show-commit -r 1:4 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
>>  	"
>>
>>  test_expect_success 'test ascending revision range with --show-commit (sha1)' "
>> @@ -74,7 +76,8 @@ test_expect_success 'test ascending revision range with --show-commit (sha1)' "
>>  	git svn find-rev r2 >>expected-range-r1-r2-r4-sha1 &&
>>  	git svn find-rev r4 >>expected-range-r1-r2-r4-sha1 &&
>>  	git reset --hard origin/trunk &&
>> -	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f2 >out &&
>> +	git svn log --show-commit -r 1:4 >out1 &&
>> +	grep '^r[0-9]' out1 | cut -d'|' -f2 >out &&
>>  	git rev-parse \$(cat out) >actual &&
>>  	test_cmp expected-range-r1-r2-r4-sha1 actual
>>  	"
>> @@ -83,45 +86,52 @@ printf 'r4 \nr2 \nr1 \n' > expected-range-r4-r2-r1
>>
>>  test_expect_success 'test descending revision range' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 4:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
>> +	git svn log -r 4:1 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
>>  	"
>>
>>  printf 'r1 \nr2 \n' > expected-range-r1-r2
>>
>>  test_expect_success 'test ascending revision range with unreachable revision' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 1:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
>> +	git svn log -r 1:3 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
>>  	"
>>
>>  printf 'r2 \nr1 \n' > expected-range-r2-r1
>>
>>  test_expect_success 'test descending revision range with unreachable revision' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 3:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
>> +	git svn log -r 3:1 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
>>  	"
>>
>>  printf 'r2 \n' > expected-range-r2
>>
>>  test_expect_success 'test ascending revision range with unreachable upper boundary revision and 1 commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 2:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
>> +	git svn log -r 2:3 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2 -
>>  	"
>>
>>  test_expect_success 'test descending revision range with unreachable upper boundary revision and 1 commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 3:2 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
>> +	git svn log -r 3:2 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r2 -
>>  	"
>>
>>  printf 'r4 \n' > expected-range-r4
>>
>>  test_expect_success 'test ascending revision range with unreachable lower boundary revision and 1 commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 3:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
>> +	git svn log -r 3:4 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>>  	"
>>
>>  test_expect_success 'test descending revision range with unreachable lower boundary revision and 1 commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 4:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
>> +	git svn log -r 4:3 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>>  	"
>>
>>  printf -- '------------------------------------------------------------------------\n' > expected-separator
>> @@ -138,12 +148,14 @@ test_expect_success 'test descending revision range with unreachable boundary re
>>
>>  test_expect_success 'test ascending revision range with unreachable boundary revisions and 1 commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 3:5 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
>> +	git svn log -r 3:5 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>>  	"
>>
>>  test_expect_success 'test descending revision range with unreachable boundary revisions and 1 commit' "
>>  	git reset --hard origin/trunk &&
>> -	git svn log -r 5:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
>> +	git svn log -r 5:3 >out &&
>> +	grep '^r[0-9]' out | cut -d'|' -f1 | test_cmp expected-range-r4 -
>>  	"
>>
>>  test_done
>> --
>> 2.26.0.rc2.28.g7fcb965970
>>
Yeah, this looks good! I will add these changes in the next patch version.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] t9116: avoid using pipes
  2020-03-22 12:46 [GSoC][PATCH 0/1] Microproject Submission Shanthanu
  2020-03-22 12:46 ` [GSoC][PATCH 1/1] t9116: avoid using pipes Shanthanu
@ 2020-03-24  9:35 ` Shanthanu
  2020-03-24 18:34   ` Christian Couder
  1 sibling, 1 reply; 7+ messages in thread
From: Shanthanu @ 2020-03-24  9:35 UTC (permalink / raw)
  To: git; +Cc: Shanthanu, tboegi

Commit c6f44e1da5 (t9813: avoid using pipes, 2017-01-04) recommends to
avoid using pipes, since the exit code of upstream in the pipe is
ignored. Hence, redirect the output to a file and parse that file.

Commit de26f02db1 (t9001, t9116: avoid pipes, 2020-02-14) noted that
this also allows easy debugging in case the test fails, since the file
will be left on disk and can be manually inspected.

Signed-off-by: Shanthanu <shanthanu.s.rai9@gmail.com>
---
 t/t9116-git-svn-log.sh | 53 +++++++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
index 0a9f1ef366..56d68e4aed 100755
--- a/t/t9116-git-svn-log.sh
+++ b/t/t9116-git-svn-log.sh
@@ -61,12 +61,16 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
 
 test_expect_success 'test ascending revision range' "
 	git reset --hard origin/trunk &&
-	git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
+	git svn log -r 1:4 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r1-r2-r4 actual
 	"
 
 test_expect_success 'test ascending revision range with --show-commit' "
 	git reset --hard origin/trunk &&
-	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
+	git svn log --show-commit -r 1:4 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r1-r2-r4 actual
 	"
 
 test_expect_success 'test ascending revision range with --show-commit (sha1)' "
@@ -74,7 +78,8 @@ test_expect_success 'test ascending revision range with --show-commit (sha1)' "
 	git svn find-rev r2 >>expected-range-r1-r2-r4-sha1 &&
 	git svn find-rev r4 >>expected-range-r1-r2-r4-sha1 &&
 	git reset --hard origin/trunk &&
-	git svn log --show-commit -r 1:4 | grep '^r[0-9]' | cut -d'|' -f2 >out &&
+	git svn log --show-commit -r 1:4 >out1 &&
+	grep '^r[0-9]' out1 | cut -d'|' -f2 >out &&
 	git rev-parse \$(cat out) >actual &&
 	test_cmp expected-range-r1-r2-r4-sha1 actual
 	"
@@ -83,67 +88,87 @@ printf 'r4 \nr2 \nr1 \n' > expected-range-r4-r2-r1
 
 test_expect_success 'test descending revision range' "
 	git reset --hard origin/trunk &&
-	git svn log -r 4:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4-r2-r1 -
+	git svn log -r 4:1 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r4-r2-r1 actual
 	"
 
 printf 'r1 \nr2 \n' > expected-range-r1-r2
 
 test_expect_success 'test ascending revision range with unreachable revision' "
 	git reset --hard origin/trunk &&
-	git svn log -r 1:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2 -
+	git svn log -r 1:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r1-r2 actual
 	"
 
 printf 'r2 \nr1 \n' > expected-range-r2-r1
 
 test_expect_success 'test descending revision range with unreachable revision' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:1 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2-r1 -
+	git svn log -r 3:1 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r2-r1 actual
 	"
 
 printf 'r2 \n' > expected-range-r2
 
 test_expect_success 'test ascending revision range with unreachable upper boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 2:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
+	git svn log -r 2:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r2 actual
 	"
 
 test_expect_success 'test descending revision range with unreachable upper boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:2 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r2 -
+	git svn log -r 3:2 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r2 actual
 	"
 
 printf 'r4 \n' > expected-range-r4
 
 test_expect_success 'test ascending revision range with unreachable lower boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 3:4 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r4 actual
 	"
 
 test_expect_success 'test descending revision range with unreachable lower boundary revision and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 4:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 4:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r4 actual
 	"
 
 printf -- '------------------------------------------------------------------------\n' > expected-separator
 
 test_expect_success 'test ascending revision range with unreachable boundary revisions and no commits' "
 	git reset --hard origin/trunk &&
-	git svn log -r 5:6 | test_cmp expected-separator -
+	git svn log -r 5:6 >actual &&
+	test_cmp expected-separator actual
 	"
 
 test_expect_success 'test descending revision range with unreachable boundary revisions and no commits' "
 	git reset --hard origin/trunk &&
-	git svn log -r 6:5 | test_cmp expected-separator -
+	git svn log -r 6:5 >actual &&
+	test_cmp expected-separator actual
 	"
 
 test_expect_success 'test ascending revision range with unreachable boundary revisions and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 3:5 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 3:5 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r4 actual
 	"
 
 test_expect_success 'test descending revision range with unreachable boundary revisions and 1 commit' "
 	git reset --hard origin/trunk &&
-	git svn log -r 5:3 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r4 -
+	git svn log -r 5:3 >out &&
+	grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
+	test_cmp expected-range-r4 actual
 	"
 
 test_done
-- 
2.26.0.rc2.28.g7fcb965970


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t9116: avoid using pipes
  2020-03-24  9:35 ` [PATCH v2] " Shanthanu
@ 2020-03-24 18:34   ` Christian Couder
  2020-03-25 14:00     ` Shanthanu
  0 siblings, 1 reply; 7+ messages in thread
From: Christian Couder @ 2020-03-24 18:34 UTC (permalink / raw)
  To: Shanthanu; +Cc: git, Torsten Bögershausen

On Tue, Mar 24, 2020 at 10:36 AM Shanthanu <shanthanu.s.rai9@gmail.com> wrote:
>
> Commit c6f44e1da5 (t9813: avoid using pipes, 2017-01-04) recommends to
> avoid using pipes, since the exit code of upstream in the pipe is
> ignored. Hence, redirect the output to a file and parse that file.
>
> Commit de26f02db1 (t9001, t9116: avoid pipes, 2020-02-14) noted that
> this also allows easy debugging in case the test fails, since the file
> will be left on disk and can be manually inspected.
>
> Signed-off-by: Shanthanu <shanthanu.s.rai9@gmail.com>

We usually ask for a legal name in a format similar to "<Firstname>
<MaybeMiddleNameInitial> <Lastname>" where <MaybeMiddleNameInitial>
can be omitted. The name should also match what is in the "From:"
field of the emails you send.

> ---
>  t/t9116-git-svn-log.sh | 53 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
> index 0a9f1ef366..56d68e4aed 100755
> --- a/t/t9116-git-svn-log.sh
> +++ b/t/t9116-git-svn-log.sh
> @@ -61,12 +61,16 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
>
>  test_expect_success 'test ascending revision range' "
>         git reset --hard origin/trunk &&
> -       git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
> +       git svn log -r 1:4 >out &&
> +       grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
> +       test_cmp expected-range-r1-r2-r4 actual
>         "

It seems that the code that your patch changes is repeated a lot in
this test script. I wonder if it would be better to write a shell
function to avoid those repetitions.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] t9116: avoid using pipes
  2020-03-24 18:34   ` Christian Couder
@ 2020-03-25 14:00     ` Shanthanu
  0 siblings, 0 replies; 7+ messages in thread
From: Shanthanu @ 2020-03-25 14:00 UTC (permalink / raw)
  To: Christian Couder; +Cc: git, Torsten Bögershausen


On 25/03/20 12:04 am, Christian Couder wrote:
> On Tue, Mar 24, 2020 at 10:36 AM Shanthanu <shanthanu.s.rai9@gmail.com> wrote:
>> Commit c6f44e1da5 (t9813: avoid using pipes, 2017-01-04) recommends to
>> avoid using pipes, since the exit code of upstream in the pipe is
>> ignored. Hence, redirect the output to a file and parse that file.
>>
>> Commit de26f02db1 (t9001, t9116: avoid pipes, 2020-02-14) noted that
>> this also allows easy debugging in case the test fails, since the file
>> will be left on disk and can be manually inspected.
>>
>> Signed-off-by: Shanthanu <shanthanu.s.rai9@gmail.com>
> We usually ask for a legal name in a format similar to "<Firstname>
> <MaybeMiddleNameInitial> <Lastname>" where <MaybeMiddleNameInitial>
> can be omitted. The name should also match what is in the "From:"
> field of the emails you send.
Okay, I will keep that in mind.
>> ---
>>  t/t9116-git-svn-log.sh | 53 +++++++++++++++++++++++++++++++-----------
>>  1 file changed, 39 insertions(+), 14 deletions(-)
>>
>> diff --git a/t/t9116-git-svn-log.sh b/t/t9116-git-svn-log.sh
>> index 0a9f1ef366..56d68e4aed 100755
>> --- a/t/t9116-git-svn-log.sh
>> +++ b/t/t9116-git-svn-log.sh
>> @@ -61,12 +61,16 @@ printf 'r1 \nr2 \nr4 \n' > expected-range-r1-r2-r4
>>
>>  test_expect_success 'test ascending revision range' "
>>         git reset --hard origin/trunk &&
>> -       git svn log -r 1:4 | grep '^r[0-9]' | cut -d'|' -f1 | test_cmp expected-range-r1-r2-r4 -
>> +       git svn log -r 1:4 >out &&
>> +       grep '^r[0-9]' out | cut -d'|' -f1 >actual &&
>> +       test_cmp expected-range-r1-r2-r4 actual
>>         "
> It seems that the code that your patch changes is repeated a lot in
> this test script. I wonder if it would be better to write a shell
> function to avoid those repetitions.

Oh yeah, that would be better. I will add a function. Just clarifying,
the function should run the git, grep, cut and test_cmp commands?

Also, could you suggest an appropriate name for this function?


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-25 14:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-22 12:46 [GSoC][PATCH 0/1] Microproject Submission Shanthanu
2020-03-22 12:46 ` [GSoC][PATCH 1/1] t9116: avoid using pipes Shanthanu
2020-03-22 17:36   ` Torsten Bögershausen
2020-03-23  7:04     ` Shanthanu
2020-03-24  9:35 ` [PATCH v2] " Shanthanu
2020-03-24 18:34   ` Christian Couder
2020-03-25 14:00     ` Shanthanu

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).