diff mbox series

[v3,4/8] t/lib-patch-mode.sh: fix ignored exit codes

Message ID patch-v3-4.8-c080899dd5f-20221202T114733Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series tests: fix ignored & hidden exit codes | expand

Commit Message

Ævar Arnfjörð Bjarmason Dec. 2, 2022, 11:52 a.m. UTC
Fix code added in b319ef70a94 (Add a small patch-mode testing library,
2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
This avoids losing both the exit code of a "git" and the "cat"
processes.

This fixes cases where we'd have e.g. missed memory leaks under
SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
discovered it while looking at leaks in related code.

The "cat _head >expect" here is redundant, we could simply give
"_head" to "test_cmp", but let's be consistent in using the "expect"
and "actual" names for clarity.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/lib-patch-mode.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

René Scharfe Dec. 2, 2022, 3:59 p.m. UTC | #1
Am 02.12.22 um 12:52 schrieb Ævar Arnfjörð Bjarmason:
> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
> 2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
> This avoids losing both the exit code of a "git" and the "cat"
> processes.
>
> This fixes cases where we'd have e.g. missed memory leaks under
> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
> discovered it while looking at leaks in related code.
>
> The "cat _head >expect" here is redundant, we could simply give
> "_head" to "test_cmp", but let's be consistent in using the "expect"
> and "actual" names for clarity.

The code at the bottom uses _head directly, which is fine IMHO, but then
this sentence should go.

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/lib-patch-mode.sh | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
> index cfd76bf987b..89ca1f78055 100644
> --- a/t/lib-patch-mode.sh
> +++ b/t/lib-patch-mode.sh
> @@ -29,8 +29,12 @@ set_and_save_state () {
>
>  # verify_state <path> <expected-worktree-content> <expected-index-content>
>  verify_state () {
> -	test "$(cat "$1")" = "$2" &&
> -	test "$(git show :"$1")" = "$3"
> +	echo "$2" >expect &&
> +	test_cmp expect "$1" &&
> +
> +	echo "$3" >expect &&
> +	git show :"$1" >actual &&
> +	test_cmp expect actual
>  }
>
>  # verify_saved_state <path>
> @@ -46,5 +50,6 @@ save_head () {
>  }
>
>  verify_saved_head () {
> -	test "$(cat _head)" = "$(git rev-parse HEAD)"
> +	git rev-parse HEAD >actual &&
> +	test_cmp _head actual
>  }
Eric Sunshine Dec. 4, 2022, 12:45 a.m. UTC | #2
On Fri, Dec 2, 2022 at 6:53 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> Fix code added in b319ef70a94 (Add a small patch-mode testing library,
> 2009-08-13) to use &&-chaining and the newly added "test_cmp_cmd".
> This avoids losing both the exit code of a "git" and the "cat"
> processes.

This still talks about test_cmp_cmd() which is no longer present in v3.

> This fixes cases where we'd have e.g. missed memory leaks under
> SANITIZE=leak, this code doesn't leak now as far as I can tell, but I
> discovered it while looking at leaks in related code.
>
> The "cat _head >expect" here is redundant, we could simply give
> "_head" to "test_cmp", but let's be consistent in using the "expect"
> and "actual" names for clarity.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
diff mbox series

Patch

diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh
index cfd76bf987b..89ca1f78055 100644
--- a/t/lib-patch-mode.sh
+++ b/t/lib-patch-mode.sh
@@ -29,8 +29,12 @@  set_and_save_state () {
 
 # verify_state <path> <expected-worktree-content> <expected-index-content>
 verify_state () {
-	test "$(cat "$1")" = "$2" &&
-	test "$(git show :"$1")" = "$3"
+	echo "$2" >expect &&
+	test_cmp expect "$1" &&
+
+	echo "$3" >expect &&
+	git show :"$1" >actual &&
+	test_cmp expect actual
 }
 
 # verify_saved_state <path>
@@ -46,5 +50,6 @@  save_head () {
 }
 
 verify_saved_head () {
-	test "$(cat _head)" = "$(git rev-parse HEAD)"
+	git rev-parse HEAD >actual &&
+	test_cmp _head actual
 }