[2/2] t6020: do not mangle trailing spaces in output

Commit Message

Jiang Xin June 1, 2021, 2:49 p.m. UTC

From: Jiang Xin <[e-mail]>

We used to call `make_user_friendly_and_stable_output` to mangle
trailing spaces in output before comparing with the expect file.
Ævar recommends generating expect file using pattern "'s/Z$//'" to
compare expect file with raw output.

Suggested-by: Ævar Arnfjörð Bjarmason <[e-mail]>
Signed-off-by: Jiang Xin <[e-mail]>
---
 t/t6020-bundle-misc.sh | 89 ++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 43 deletions(-)

Comments

Ævar Arnfjörð Bjarmason June 5, 2021, 5:02 p.m. UTC | #1

On Tue, Jun 01 2021, Jiang Xin wrote:

> From: Jiang Xin <[e-mail]>
>
> We used to call `make_user_friendly_and_stable_output` to mangle
> trailing spaces in output before comparing with the expect file.
> Ævar recommends generating expect file using pattern "'s/Z$//'" to
> compare expect file with raw output.

I've tested both of these patches and they fix the reported issue on
that gcc45 machine.

> Suggested-by: Ævar Arnfjörð Bjarmason <[e-mail]>
> Signed-off-by: Jiang Xin <[e-mail]>
> ---
>  t/t6020-bundle-misc.sh | 89 ++++++++++++++++++++++--------------------
>  1 file changed, 46 insertions(+), 43 deletions(-)
>
> diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
> index c6a8ea7f76..7ee43fb1df 100755
> --- a/t/t6020-bundle-misc.sh
> +++ b/t/t6020-bundle-misc.sh
> @@ -83,7 +83,7 @@ test_commit_setvar () {
>  
>  # Format the output of git commands to make a user-friendly and stable
>  # text.  We can easily prepare the expect text without having to worry
> -# about future changes of the commit ID and spaces of the output.
> +# about future changes of the commit ID.
>  make_user_friendly_and_stable_output () {
>  	sed \
>  		-e "s/$A[0-9a-f]*/<COMMIT-A>/g" \
> @@ -104,8 +104,11 @@ make_user_friendly_and_stable_output () {
>  		-e "s/$P[0-9a-f]*/<COMMIT-P>/g" \
>  		-e "s/$TAG1[0-9a-f]*/<TAG-1>/g" \
>  		-e "s/$TAG2[0-9a-f]*/<TAG-2>/g" \
> -		-e "s/$TAG3[0-9a-f]*/<TAG-3>/g" \
> -		-e "s/ *\$//"
> +		-e "s/$TAG3[0-9a-f]*/<TAG-3>/g"
> +}
> +
> +format_and_save_expect () {
> +	sed -e 's/^> //' -e 's/Z$//' >expect
>  }
>  
>  #            (C)   (D, pull/1/head, topic/1)
> @@ -180,11 +183,11 @@ test_expect_success 'create bundle from special rev: main^!' '
>  
>  	git bundle verify special-rev.bdl |
>  		make_user_friendly_and_stable_output >actual &&
> -	cat >expect <<-\EOF &&
> -	The bundle contains this ref:
> -	<COMMIT-P> refs/heads/main
> -	The bundle requires this ref:
> -	<COMMIT-O>
> +	format_and_save_expect <<-\EOF &&
> +	> The bundle contains this ref:
> +	> <COMMIT-P> refs/heads/main
> +	> The bundle requires this ref:
> +	> <COMMIT-O> Z
>  	EOF
>  	test_cmp expect actual &&

I think for this test it would be better to just have the "Z" suffix and
leave off prefixing all the lines with ">". That's done in t0000.sh
because we have various empty lines, but here that's not the case.

I don't think that nit is worth a re-roll, and as you noted you're doing
some larger changes to these tests in follow-up patches.

It's just convenient to have the test_cmp expect input as close to
copy/paste-able as possible.

Patch

diff --git a/t/t6020-bundle-misc.sh b/t/t6020-bundle-misc.sh
index c6a8ea7f76..7ee43fb1df 100755
--- a/t/t6020-bundle-misc.sh
+++ b/t/t6020-bundle-misc.sh
@@ -83,7 +83,7 @@  test_commit_setvar () {
 
 # Format the output of git commands to make a user-friendly and stable
 # text.  We can easily prepare the expect text without having to worry
-# about future changes of the commit ID and spaces of the output.
+# about future changes of the commit ID.
 make_user_friendly_and_stable_output () {
 	sed \
 		-e "s/$A[0-9a-f]*/<COMMIT-A>/g" \
@@ -104,8 +104,11 @@  make_user_friendly_and_stable_output () {
 		-e "s/$P[0-9a-f]*/<COMMIT-P>/g" \
 		-e "s/$TAG1[0-9a-f]*/<TAG-1>/g" \
 		-e "s/$TAG2[0-9a-f]*/<TAG-2>/g" \
-		-e "s/$TAG3[0-9a-f]*/<TAG-3>/g" \
-		-e "s/ *\$//"
+		-e "s/$TAG3[0-9a-f]*/<TAG-3>/g"
+}
+
+format_and_save_expect () {
+	sed -e 's/^> //' -e 's/Z$//' >expect
 }
 
 #            (C)   (D, pull/1/head, topic/1)
@@ -180,11 +183,11 @@  test_expect_success 'create bundle from special rev: main^!' '
 
 	git bundle verify special-rev.bdl |
 		make_user_friendly_and_stable_output >actual &&
-	cat >expect <<-\EOF &&
-	The bundle contains this ref:
-	<COMMIT-P> refs/heads/main
-	The bundle requires this ref:
-	<COMMIT-O>
+	format_and_save_expect <<-\EOF &&
+	> The bundle contains this ref:
+	> <COMMIT-P> refs/heads/main
+	> The bundle requires this ref:
+	> <COMMIT-O> Z
 	EOF
 	test_cmp expect actual &&
 
@@ -201,12 +204,12 @@  test_expect_success 'create bundle with --max-count option' '
 
 	git bundle verify max-count.bdl |
 		make_user_friendly_and_stable_output >actual &&
-	cat >expect <<-\EOF &&
-	The bundle contains these 2 refs:
-	<COMMIT-P> refs/heads/main
-	<TAG-1> refs/tags/v1
-	The bundle requires this ref:
-	<COMMIT-O>
+	format_and_save_expect <<-\EOF &&
+	> The bundle contains these 2 refs:
+	> <COMMIT-P> refs/heads/main
+	> <TAG-1> refs/tags/v1
+	> The bundle requires this ref:
+	> <COMMIT-O> Z
 	EOF
 	test_cmp expect actual &&
 
@@ -226,16 +229,16 @@  test_expect_success 'create bundle with --since option' '
 
 	git bundle verify since.bdl |
 		make_user_friendly_and_stable_output >actual &&
-	cat >expect <<-\EOF &&
-	The bundle contains these 5 refs:
-	<COMMIT-P> refs/heads/main
-	<COMMIT-N> refs/heads/release
-	<TAG-2> refs/tags/v2
-	<TAG-3> refs/tags/v3
-	<COMMIT-P> HEAD
-	The bundle requires these 2 refs:
-	<COMMIT-M>
-	<COMMIT-K>
+	format_and_save_expect <<-\EOF &&
+	> The bundle contains these 5 refs:
+	> <COMMIT-P> refs/heads/main
+	> <COMMIT-N> refs/heads/release
+	> <TAG-2> refs/tags/v2
+	> <TAG-3> refs/tags/v3
+	> <COMMIT-P> HEAD
+	> The bundle requires these 2 refs:
+	> <COMMIT-M> Z
+	> <COMMIT-K> Z
 	EOF
 	test_cmp expect actual &&
 
@@ -294,13 +297,13 @@  test_expect_success 'create bundle 2 - has prerequisites' '
 		--stdin \
 		release <input &&
 
-	cat >expect <<-\EOF &&
-	The bundle contains this ref:
-	<COMMIT-N> refs/heads/release
-	The bundle requires these 3 refs:
-	<COMMIT-D>
-	<COMMIT-E>
-	<COMMIT-G>
+	format_and_save_expect <<-\EOF &&
+	> The bundle contains this ref:
+	> <COMMIT-N> refs/heads/release
+	> The bundle requires these 3 refs:
+	> <COMMIT-D> Z
+	> <COMMIT-E> Z
+	> <COMMIT-G> Z
 	EOF
 
 	git bundle verify 2.bdl |
@@ -318,11 +321,11 @@  test_expect_success 'create bundle 2 - has prerequisites' '
 test_expect_success 'fail to verify bundle without prerequisites' '
 	git init --bare test1.git &&
 
-	cat >expect <<-\EOF &&
-	error: Repository lacks these prerequisite commits:
-	error: <COMMIT-D>
-	error: <COMMIT-E>
-	error: <COMMIT-G>
+	format_and_save_expect <<-\EOF &&
+	> error: Repository lacks these prerequisite commits:
+	> error: <COMMIT-D> Z
+	> error: <COMMIT-E> Z
+	> error: <COMMIT-G> Z
 	EOF
 
 	test_must_fail git -C test1.git bundle verify ../2.bdl 2>&1 |
@@ -353,13 +356,13 @@  test_expect_success 'create bundle 3 - two refs, same object' '
 		--stdin \
 		main HEAD <input &&
 
-	cat >expect <<-\EOF &&
-	The bundle contains these 2 refs:
-	<COMMIT-P> refs/heads/main
-	<COMMIT-P> HEAD
-	The bundle requires these 2 refs:
-	<COMMIT-M>
-	<COMMIT-K>
+	format_and_save_expect <<-\EOF &&
+	> The bundle contains these 2 refs:
+	> <COMMIT-P> refs/heads/main
+	> <COMMIT-P> HEAD
+	> The bundle requires these 2 refs:
+	> <COMMIT-M> Z
+	> <COMMIT-K> Z
 	EOF
 
 	git bundle verify 3.bdl |