git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] t0030-t0050: avoid pipes with Git on LHS

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Feb 24, 2022, 08:59 UTC
Message-ID
<220224.86o82wab31.gmgdl@evledraar.gmail.com>
In-Reply-To
<20220224054720.23996-3-shivam828787@gmail.com>
On Thu, Feb 24 2022, Shubham Mishra wrote:
Show 31 quoted lines
> Pipes ignore error codes of LHS command and thus we should not use
> them with Git in tests. As an alternative, use a 'tmp' file to write
> the Git output so we can test the exit code.
>
> Signed-off-by: Shubham Mishra <shivam828787@gmail.com>
> ---
>  t/t0030-stripspace.sh | 81 ++++++++++++++++++++++++++++---------------
>  t/t0050-filesystem.sh |  3 +-
>  2 files changed, 56 insertions(+), 28 deletions(-)
>
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index ae1ca380c1..4d0af9bf98 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -225,32 +225,48 @@ test_expect_success \
>  
>  test_expect_success \
>      'text without newline at end should end with newline' '
> -    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
> +    printf "$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$ttt$ttt" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0
>  '

You're modifying some tests here that are using some old coding style, so maybe it's better to adjust it while we're at it?

Also I think this would be a lot nicer with test_stdout_line_count and a
helper to deal with that pritnf, e.g.:
	
	diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
	index ae1ca380c1a..d48a3579511 100755
	--- a/t/t0030-stripspace.sh
	+++ b/t/t0030-stripspace.sh
	@@ -223,12 +223,15 @@ test_expect_success \
	     test_cmp expect actual
	 '
	 
	-test_expect_success \
	-    'text without newline at end should end with newline' '
	-    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
	-    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
	-    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
	-    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
	+printf_git_stripspace () {
	+	printf "$1" | git stripspace
	+}
	+
	+test_expect_success 'text without newline at end should end with newline' '
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt" &&
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt" &&
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt" &&
	+	test_stdout_line_count -gt 0 printf_git_stripspace "$ttt$ttt$ttt$ttt"
	 '
	 
	 # text plus spaces at the end:
Show 23 quoted lines
>  # text plus spaces at the end:
>  
>  test_expect_success \
>      'text plus spaces without newline at end should end with newline' '
> -    test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
> -    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
> +    printf "$ttt$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$ttt$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$sss$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$ttt$sss$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0 &&
> +    printf "$ttt$sss$sss$sss" | git stripspace >tmp &&
> +    test $(wc -l <tmp) -gt 0
>  '
ditto.
Show 21 quoted lines
>  test_expect_success \
>      'text plus spaces without newline at end should not show spaces' '
> -    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
> +    printf "$ttt$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$ttt$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$ttt$ttt$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/nul) &&
> +    printf "$ttt$sss$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$ttt$sss$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    printf "$ttt$sss$sss$sss" | git stripspace &&
> +    ! (grep "  " tmp >/dev/null)
>  '

This is not on you, but generally we don't pipe "grep" to >/dev/null, and just let the --verbose option do its work.

With/without that change you no longer need the () subshell here.
Show 46 quoted lines
>  test_expect_success \
> @@ -282,12 +298,18 @@ test_expect_success \
>  
>  test_expect_success \
>      'text plus spaces at end should not show spaces' '
> -    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
> -    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
> +    echo "$ttt$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$ttt$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp>/dev/null) &&
> +    echo "$ttt$ttt$ttt$sss" &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$sss$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$ttt$sss$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null) &&
> +    echo "$ttt$sss$sss$sss" | git stripspace >tmp &&
> +    ! (grep "  " tmp >/dev/null)
>  '
>  
>  test_expect_success \
> @@ -339,11 +361,16 @@ test_expect_success \
>  
>  test_expect_success \
>      'spaces without newline at end should not show spaces' '
> -    ! (printf "" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
> -    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
> +    printf "" | git stripspace >tmp &&
> +    ! ( grep " " tmp >/dev/null) &&
> +    printf "$sss" | git stripspace >tmp &&
> +    ! ( grep " " tmp >/dev/null) &&
> +    printf "$sss$sss" | git stripspace >tmp &&
> +    ! (grep " " tmp >/dev/null) &&
> +    printf "$sss$sss$sss" | git stripspace >tmp &&
> +    ! (grep " " tmp >/dev/null) &&
> +    printf "$sss$sss$sss$sss" | git stripspace >tmp &&
> +    ! (grep " " tmp >/dev/null)
>  '
ditto for these.
Show 15 quoted lines
>  test_expect_success \
> diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
> index afc343cf9b..5c9dc90d0b 100755
> --- a/t/t0050-filesystem.sh
> +++ b/t/t0050-filesystem.sh
> @@ -104,7 +104,8 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
>  	rm camelcase &&
>  	echo 1 >CamelCase &&
>  	git add CamelCase &&
> -	camel=$(git ls-files | grep -i camelcase) &&
> +	git ls-files >tmp &&
> +	camel=$(grep -i camelcase tmp) &&
>  	test $(echo "$camel" | wc -l) = 1 &&
>  	test "z$(git cat-file blob :$camel)" = z1
>  '
Previous: Shubham MishraNext: Shubham Mishra
Message 4 of 19 in “Subject: [PATCH 0/2] [GSoC][PATCH 0/2] t0000-t0050: avoid pipes with Git on LHS”
  1. 0/2 Subject: [PATCH 0/2] [GSoC][PATCH 0/2] t0000-t0050: avoid pipes with Git on LHSShubham Mishra, Feb 24, 2022
  2. 1/2 t0001-t0028: avoid pipes with Git on LHSShubham Mishra, Feb 24, 2022
  3. 2/2 t0030-t0050: avoid pipes with Git on LHSShubham Mishra, Feb 24, 2022
  4. Ævar Arnfjörð BjarmasonFeb 24, 2022
  5. Shubham MishraFeb 26, 2022
  6. Taylor BlauFeb 26, 2022
  7. [GSoC] [PATCH v2 0/2] avoid pipes with Git on LHSShubham Mishra, Feb 27, 2022
  8. 1/2 t0001-t0028: avoid pipes with Git on LHSShubham Mishra, Feb 27, 2022
  9. Johannes SixtFeb 27, 2022
  10. Shubham MishraMar 3, 2022
  11. 2/2 t0030-t0050: avoid pipes with Git on LHSShubham Mishra, Feb 27, 2022
  12. Johannes SixtFeb 27, 2022
  13. Shubham MishraFeb 24, 2022
  14. Taylor BlauFeb 26, 2022
  15. [GSoC] [PATCH v3 0/2] avoid pipes with Git on LHSShubham Mishra, Mar 12, 2022
  16. 1/2 t0001-t0028: avoid pipes with Git on LHSShubham Mishra, Mar 12, 2022
  17. 2/2 t0030-t0050: avoid pipes with Git on LHSShubham Mishra, Mar 12, 2022
  18. Johannes SixtMar 12, 2022
  19. Shubham MishraMar 12, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.