[PATCH 0/2] t5403: improve post-checkout hook testing

[PATCH 0/2] t5403: improve post-checkout hook testing

[Deveshi Dwivedi]

Following up on the recently merged patch that replaced 'test -f' with
test_path_is_file, this series continues the cleanup of the
post-checkout hook tests.

Patch 1/2 introduces a check_post_checkout helper function to replace
the repeated pattern of reading and validating hook arguments. This
refactoring does not change test behavior, but makes the code easier
to maintain and prepares it for further improvement.

Patch 2/2 updates the helper and hook output format to use test_cmp
instead of individual test commands. This provides clearer error
messages on failure, making it easier to see which argument (old ref,
new ref, or flag) did not match the expectation.


Deveshi Dwivedi (2):
  t5403:introduce check_post_checkout helper function
  t5403: use test_cmp for post-checkout argument checks

 t/t5403-post-checkout-hook.sh | 49 +++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 23 deletions(-)

-- 
2.52.0.230.gd8af7cadaa

[PATCH 1/2] t5403:introduce check_post_checkout helper function

[Deveshi Dwivedi]

The test file repeatedly uses the same four-line pattern to validate
post-checkout hook arguments: read the args file, then test each of
the three values individually.

Introduce a check_post_checkout helper function that encapsulates this
pattern. This patch does not change test behavior; it prepares the
code for improvement in the next step.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 45 +++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 1462e3365b..63a2221441 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,6 +9,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+# Helper function to check post-checkout hook arguments
+check_post_checkout () {
+	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
+	read old new flag <"$1" &&
+	test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
+}
+
 test_expect_success setup '
 	test_hook --setup post-checkout <<-\EOF &&
 	echo "$@" >.git/post-checkout.args
@@ -23,29 +30,30 @@ test_expect_success setup '
 test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout main &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout args are correct with git checkout -b ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout -b new1 &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout receives the right args with HEAD changed ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
+	old=$(git rev-parse HEAD) &&
 	git checkout two &&
-	read old new flag <.git/post-checkout.args &&
-	test $old != $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$old" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout receives the right args when not switching branches ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout main -- three.t &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 0
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 0
 '
 
 test_rebase () {
@@ -55,10 +63,8 @@ test_rebase () {
 		git checkout -B rebase-test main &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args rebase-on-me &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev main $old &&
-		test_cmp_rev rebase-on-me $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse main)" "$(git rev-parse rebase-on-me)" 1
 	'
 
 	test_expect_success "post-checkout is triggered on rebase $args with fast-forward" '
@@ -66,10 +72,8 @@ test_rebase () {
 		git checkout -B ff-rebase-test rebase-on-me^ &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args rebase-on-me &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev rebase-on-me^ $old &&
-		test_cmp_rev rebase-on-me $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse rebase-on-me^)" "$(git rev-parse rebase-on-me)" 1
 	'
 
 	test_expect_success "rebase $args fast-forward branch checkout runs post-checkout hook" '
@@ -79,10 +83,8 @@ test_rebase () {
 		git checkout two  &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args HEAD rebase-fast-forward  &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev two $old &&
-		test_cmp_rev three $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse two)" "$(git rev-parse three)" 1
 	'
 
 	test_expect_success "rebase $args checkout does not remove untracked files" '
@@ -109,7 +111,8 @@ test_expect_success 'post-checkout hook is triggered by clone' '
 	echo "$@" >"$GIT_DIR/post-checkout.args"
 	EOF
 	git clone --template=templates . clone3 &&
-	test_path_is_file clone3/.git/post-checkout.args
+	check_post_checkout clone3/.git/post-checkout.args \
+		"$(test_oid zero)" "$(git -C clone3 rev-parse HEAD)" 1
 '
 
 test_done
-- 
2.52.0.230.gd8af7cadaa

Re: [PATCH 1/2] t5403:introduce check_post_checkout helper function

[Eric Sunshine]

On Sun, Jan 11, 2026 at 2:30 AM Deveshi Dwivedi
<deveshigurgaon@gmail.com> wrote:
> The test file repeatedly uses the same four-line pattern to validate
> post-checkout hook arguments: read the args file, then test each of
> the three values individually.
>
> Introduce a check_post_checkout helper function that encapsulates this
> pattern. This patch does not change test behavior; it prepares the
> code for improvement in the next step.
>
> Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
> ---
> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> @@ -9,6 +9,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> +# Helper function to check post-checkout hook arguments
> +check_post_checkout () {
> +       test "$#" = 4 || BUG "check_post_checkout takes 4 args"
> +       read old new flag <"$1" &&
> +       test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
> +}

Rather than forcing people to read the function body to divine the
purpose of the four arguments, the function comment should spell out
their meaning. See the many "Usage:" comments in
t/test-lib-functions.sh for examples of how to write more useful
function documentation.

Re: [PATCH 1/2] t5403:introduce check_post_checkout helper function

[Pushkar Singh]

I agree with Eric’s point about documenting the helper’s arguments.

Since patch 2 also changes the hook output format to a structured
"old=… new=… flag=…" layout that "check_post_checkout()" depends on,
it would be especially helpful if the function comment spelled out both
the meaning of the four parameters and the expected on-disk format of
the args file.

That would make the helper’s contract much clearer to future readers
and reduce the risk of accidental breakage if the hook output changes.

On Sun, Jan 11, 2026 at 1:23 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sun, Jan 11, 2026 at 2:30 AM Deveshi Dwivedi
> <deveshigurgaon@gmail.com> wrote:
> > The test file repeatedly uses the same four-line pattern to validate
> > post-checkout hook arguments: read the args file, then test each of
> > the three values individually.
> >
> > Introduce a check_post_checkout helper function that encapsulates this
> > pattern. This patch does not change test behavior; it prepares the
> > code for improvement in the next step.
> >
> > Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
> > ---
> > diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> > @@ -9,6 +9,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
> > +# Helper function to check post-checkout hook arguments
> > +check_post_checkout () {
> > +       test "$#" = 4 || BUG "check_post_checkout takes 4 args"
> > +       read old new flag <"$1" &&
> > +       test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
> > +}
>
> Rather than forcing people to read the function body to divine the
> purpose of the four arguments, the function comment should spell out
> their meaning. See the many "Usage:" comments in
> t/test-lib-functions.sh for examples of how to write more useful
> function documentation.
>

Re: [PATCH 1/2] t5403:introduce check_post_checkout helper function

[Deveshi Dwivedi]

> Rather than forcing people to read the function body to divine the
> purpose of the four arguments, the function comment should spell out
> their meaning. See the many "Usage:" comments in
> t/test-lib-functions.sh for examples of how to write more useful
> function documentation.
Thanks for the suggestion! I agree, I'll send a v2 with better
function documentation.

[PATCH] t5403: document check_post_checkout helper

[Pushkar Singh]

---
 t/t5403-post-checkout-hook.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 63a2221441..31ad369b3f 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,7 +9,11 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
-# Helper function to check post-checkout hook arguments
+# Usage: check_post_checkout <args-file> <old> <new> <flag>
+#
+# Verifies that the post-checkout hook wrote the expected old and new
+# object IDs and the expected flag (1 for branch switch, 0 otherwise)
+# into <args-file>.
 check_post_checkout () {
 	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
 	read old new flag <"$1" &&
-- 
2.43.0

[PATCH 2/2] t5403: use test_cmp for post-checkout argument checks

[Deveshi Dwivedi]

Update check_post_checkout and the post-checkout hook implementation to
use test_cmp instead of individual test commands. This provides better
error messages when tests fail, making it easier to debug which specific
argument (old ref, new ref, or flag) was incorrect.

The hook now outputs in key=value format which test_cmp can display
clearly when there's a mismatch.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 63a2221441..a50c8d0eee 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -12,13 +12,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 # Helper function to check post-checkout hook arguments
 check_post_checkout () {
 	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
-	read old new flag <"$1" &&
-	test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
+	echo "old=$2 new=$3 flag=$4" >expect &&
+	test_cmp expect "$1"
 }
 
 test_expect_success setup '
 	test_hook --setup post-checkout <<-\EOF &&
-	echo "$@" >.git/post-checkout.args
+	echo "old=$1 new=$2 flag=$3" >.git/post-checkout.args
 	EOF
 	test_commit one &&
 	test_commit two &&
@@ -108,7 +108,7 @@ test_rebase --merge
 test_expect_success 'post-checkout hook is triggered by clone' '
 	mkdir -p templates/hooks &&
 	write_script templates/hooks/post-checkout <<-\EOF &&
-	echo "$@" >"$GIT_DIR/post-checkout.args"
+	echo "old=$1 new=$2 flag=$3" >"$GIT_DIR/post-checkout.args"
 	EOF
 	git clone --template=templates . clone3 &&
 	check_post_checkout clone3/.git/post-checkout.args \
-- 
2.52.0.230.gd8af7cadaa

[PATCH v2 0/2] t5403: improve post-checkout hook testing

[Deveshi Dwivedi]

Following up on the recently merged patch that replaced 'test -f' with
test_path_is_file, this series continues the cleanup of the
post-checkout hook tests.

Patch 1/2 introduces a check_post_checkout helper function to replace
the repeated pattern of reading and validating hook arguments. This
refactoring does not change test behavior, but makes the code easier
to maintain and prepares it for further improvement.

Patch 2/2 updates the helper and hook output format to use test_cmp
instead of individual test commands. This provides clearer error
messages on failure, making it easier to see which argument (old ref,
new ref, or flag) did not match the expectation

Changes since v1:
- Updated the check_post_checkout helper comment to be more descriptive.

Deveshi Dwivedi (2):
  t5403:introduce check_post_checkout helper function
  t5403: use test_cmp for post-checkout argument checks

 t/t5403-post-checkout-hook.sh | 53 ++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 23 deletions(-)

-- 
2.52.0.230.gd8af7cadaa

[PATCH v2 1/2] t5403:introduce check_post_checkout helper function

[Deveshi Dwivedi]

The test file repeatedly uses the same four-line pattern to validate
post-checkout hook arguments: read the args file, then test each of
the three values individually.

Introduce a check_post_checkout helper function that encapsulates this
pattern. This patch does not change test behavior; it prepares the
code for improvement in the next step.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 49 ++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 1462e3365b..7bdea25107 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,6 +9,17 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+# Usage: check_post_checkout <file> <old-ref> <new-ref> <flag>
+#
+# Verify that the post-checkout hook arguments in <file> match the expected
+# values: <old-ref> for the previous HEAD, <new-ref> for the new HEAD, and
+# <flag> indicating whether this was a branch checkout (1) or file checkout (0).
+check_post_checkout () {
+	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
+	read old new flag <"$1" &&
+	test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
+}
+
 test_expect_success setup '
 	test_hook --setup post-checkout <<-\EOF &&
 	echo "$@" >.git/post-checkout.args
@@ -23,29 +34,30 @@ test_expect_success setup '
 test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout main &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout args are correct with git checkout -b ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout -b new1 &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout receives the right args with HEAD changed ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
+	old=$(git rev-parse HEAD) &&
 	git checkout two &&
-	read old new flag <.git/post-checkout.args &&
-	test $old != $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$old" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout receives the right args when not switching branches ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout main -- three.t &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 0
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 0
 '
 
 test_rebase () {
@@ -55,10 +67,8 @@ test_rebase () {
 		git checkout -B rebase-test main &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args rebase-on-me &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev main $old &&
-		test_cmp_rev rebase-on-me $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse main)" "$(git rev-parse rebase-on-me)" 1
 	'
 
 	test_expect_success "post-checkout is triggered on rebase $args with fast-forward" '
@@ -66,10 +76,8 @@ test_rebase () {
 		git checkout -B ff-rebase-test rebase-on-me^ &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args rebase-on-me &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev rebase-on-me^ $old &&
-		test_cmp_rev rebase-on-me $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse rebase-on-me^)" "$(git rev-parse rebase-on-me)" 1
 	'
 
 	test_expect_success "rebase $args fast-forward branch checkout runs post-checkout hook" '
@@ -79,10 +87,8 @@ test_rebase () {
 		git checkout two  &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args HEAD rebase-fast-forward  &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev two $old &&
-		test_cmp_rev three $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse two)" "$(git rev-parse three)" 1
 	'
 
 	test_expect_success "rebase $args checkout does not remove untracked files" '
@@ -109,7 +115,8 @@ test_expect_success 'post-checkout hook is triggered by clone' '
 	echo "$@" >"$GIT_DIR/post-checkout.args"
 	EOF
 	git clone --template=templates . clone3 &&
-	test_path_is_file clone3/.git/post-checkout.args
+	check_post_checkout clone3/.git/post-checkout.args \
+		"$(test_oid zero)" "$(git -C clone3 rev-parse HEAD)" 1
 '
 
 test_done
-- 
2.52.0.230.gd8af7cadaa

Re: [PATCH v2 1/2] t5403:introduce check_post_checkout helper function

[Junio C Hamano]

Deveshi Dwivedi <deveshigurgaon@gmail.com> writes:

> The test file repeatedly uses the same four-line pattern to validate
> post-checkout hook arguments: read the args file, then test each of
> the three values individually.
>
> Introduce a check_post_checkout helper function that encapsulates this
> pattern. This patch does not change test behavior; it prepares the
> code for improvement in the next step.
>
> Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
> ---
>  t/t5403-post-checkout-hook.sh | 49 ++++++++++++++++++++---------------
>  1 file changed, 28 insertions(+), 21 deletions(-)

OK.

> diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
> index 1462e3365b..7bdea25107 100755
> --- a/t/t5403-post-checkout-hook.sh
> +++ b/t/t5403-post-checkout-hook.sh
> ...
> @@ -109,7 +115,8 @@ test_expect_success 'post-checkout hook is triggered by clone' '
>  	echo "$@" >"$GIT_DIR/post-checkout.args"
>  	EOF
>  	git clone --template=templates . clone3 &&
> -	test_path_is_file clone3/.git/post-checkout.args
> +	check_post_checkout clone3/.git/post-checkout.args \
> +		"$(test_oid zero)" "$(git -C clone3 rev-parse HEAD)" 1
>  '

All other hunks are as described but this one is slightly different,
which may want a mention in the proposed log message.  We used to
only care about the fact that post-checkout hook was executed, but
now we check that we invoke the hook with expected parameters.

Other than that, this looks good; so does the next step.

Thanks.

[PATCH v2 2/2] t5403: use test_cmp for post-checkout argument checks

[Deveshi Dwivedi]

Update check_post_checkout and the post-checkout hook implementation to
use test_cmp instead of individual test commands. This provides better
error messages when tests fail, making it easier to debug which specific
argument (old ref, new ref, or flag) was incorrect.

The hook now outputs in key=value format which test_cmp can display
clearly when there's a mismatch.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 7bdea25107..cb0300b2d2 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -16,13 +16,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 # <flag> indicating whether this was a branch checkout (1) or file checkout (0).
 check_post_checkout () {
 	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
-	read old new flag <"$1" &&
-	test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
+	echo "old=$2 new=$3 flag=$4" >expect &&
+	test_cmp expect "$1"
 }
 
 test_expect_success setup '
 	test_hook --setup post-checkout <<-\EOF &&
-	echo "$@" >.git/post-checkout.args
+	echo "old=$1 new=$2 flag=$3" >.git/post-checkout.args
 	EOF
 	test_commit one &&
 	test_commit two &&
@@ -112,7 +112,7 @@ test_rebase --merge
 test_expect_success 'post-checkout hook is triggered by clone' '
 	mkdir -p templates/hooks &&
 	write_script templates/hooks/post-checkout <<-\EOF &&
-	echo "$@" >"$GIT_DIR/post-checkout.args"
+	echo "old=$1 new=$2 flag=$3" >"$GIT_DIR/post-checkout.args"
 	EOF
 	git clone --template=templates . clone3 &&
 	check_post_checkout clone3/.git/post-checkout.args \
-- 
2.52.0.230.gd8af7cadaa

[PATCH v3 0/2] t5403: improve post-checkout hook testing

[Deveshi Dwivedi]

Following up on the recently merged patch that replaced 'test -f' with
test_path_is_file, this series continues the cleanup of the
post-checkout hook tests.

Patch 1/2 introduces a check_post_checkout helper function to replace
the repeated pattern of reading and validating hook arguments. This
refactoring does not change test behavior, but makes the code easier
to maintain and prepares it for further improvement.

Patch 2/2 updates the helper and hook output format to use test_cmp
instead of individual test commands. This provides clearer error
messages on failure, making it easier to see which argument (old ref,
new ref, or flag) did not match the expectation

Changes since v2:
- Update commit message of patch 1/2 to note that the clone test now
validates post-checkout hook arguments, not just hook execution.


Deveshi Dwivedi (2):
  t5403:introduce check_post_checkout helper function
  t5403: use test_cmp for post-checkout argument checks

 t/t5403-post-checkout-hook.sh | 53 ++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 23 deletions(-)

-- 
2.52.0.230.gd8af7cadaa

[PATCH v3 1/2] t5403:introduce check_post_checkout helper function

[Deveshi Dwivedi]

The test file repeatedly uses the same four-line pattern to validate
post-checkout hook arguments: read the args file, then test each of
the three values individually.

Introduce a check_post_checkout helper function that encapsulates this
pattern. This patch does not change test behavior; it prepares the
code for improvement in the next step.

Additionally, the 'post-checkout hook is triggered by clone' test is
improved to validate the hook arguments (old ref, new ref, and flag)
rather than just checking that the hook file was created.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 49 ++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 1462e3365b..7bdea25107 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -9,6 +9,17 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 . ./test-lib.sh
 
+# Usage: check_post_checkout <file> <old-ref> <new-ref> <flag>
+#
+# Verify that the post-checkout hook arguments in <file> match the expected
+# values: <old-ref> for the previous HEAD, <new-ref> for the new HEAD, and
+# <flag> indicating whether this was a branch checkout (1) or file checkout (0).
+check_post_checkout () {
+	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
+	read old new flag <"$1" &&
+	test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
+}
+
 test_expect_success setup '
 	test_hook --setup post-checkout <<-\EOF &&
 	echo "$@" >.git/post-checkout.args
@@ -23,29 +34,30 @@ test_expect_success setup '
 test_expect_success 'post-checkout receives the right arguments with HEAD unchanged ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout main &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout args are correct with git checkout -b ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout -b new1 &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout receives the right args with HEAD changed ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
+	old=$(git rev-parse HEAD) &&
 	git checkout two &&
-	read old new flag <.git/post-checkout.args &&
-	test $old != $new && test $flag = 1
+	check_post_checkout .git/post-checkout.args \
+		"$old" "$(git rev-parse HEAD)" 1
 '
 
 test_expect_success 'post-checkout receives the right args when not switching branches ' '
 	test_when_finished "rm -f .git/post-checkout.args" &&
 	git checkout main -- three.t &&
-	read old new flag <.git/post-checkout.args &&
-	test $old = $new && test $flag = 0
+	check_post_checkout .git/post-checkout.args \
+		"$(git rev-parse HEAD)" "$(git rev-parse HEAD)" 0
 '
 
 test_rebase () {
@@ -55,10 +67,8 @@ test_rebase () {
 		git checkout -B rebase-test main &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args rebase-on-me &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev main $old &&
-		test_cmp_rev rebase-on-me $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse main)" "$(git rev-parse rebase-on-me)" 1
 	'
 
 	test_expect_success "post-checkout is triggered on rebase $args with fast-forward" '
@@ -66,10 +76,8 @@ test_rebase () {
 		git checkout -B ff-rebase-test rebase-on-me^ &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args rebase-on-me &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev rebase-on-me^ $old &&
-		test_cmp_rev rebase-on-me $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse rebase-on-me^)" "$(git rev-parse rebase-on-me)" 1
 	'
 
 	test_expect_success "rebase $args fast-forward branch checkout runs post-checkout hook" '
@@ -79,10 +87,8 @@ test_rebase () {
 		git checkout two  &&
 		rm -f .git/post-checkout.args &&
 		git rebase $args HEAD rebase-fast-forward  &&
-		read old new flag <.git/post-checkout.args &&
-		test_cmp_rev two $old &&
-		test_cmp_rev three $new &&
-		test $flag = 1
+		check_post_checkout .git/post-checkout.args \
+			"$(git rev-parse two)" "$(git rev-parse three)" 1
 	'
 
 	test_expect_success "rebase $args checkout does not remove untracked files" '
@@ -109,7 +115,8 @@ test_expect_success 'post-checkout hook is triggered by clone' '
 	echo "$@" >"$GIT_DIR/post-checkout.args"
 	EOF
 	git clone --template=templates . clone3 &&
-	test_path_is_file clone3/.git/post-checkout.args
+	check_post_checkout clone3/.git/post-checkout.args \
+		"$(test_oid zero)" "$(git -C clone3 rev-parse HEAD)" 1
 '
 
 test_done
-- 
2.52.0.230.gd8af7cadaa

[PATCH v3 2/2] t5403: use test_cmp for post-checkout argument checks

[Deveshi Dwivedi]

Update check_post_checkout and the post-checkout hook implementation to
use test_cmp instead of individual test commands. This provides better
error messages when tests fail, making it easier to debug which specific
argument (old ref, new ref, or flag) was incorrect.

The hook now outputs in key=value format which test_cmp can display
clearly when there's a mismatch.

Signed-off-by: Deveshi Dwivedi <deveshigurgaon@gmail.com>
---
 t/t5403-post-checkout-hook.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t5403-post-checkout-hook.sh b/t/t5403-post-checkout-hook.sh
index 7bdea25107..cb0300b2d2 100755
--- a/t/t5403-post-checkout-hook.sh
+++ b/t/t5403-post-checkout-hook.sh
@@ -16,13 +16,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 # <flag> indicating whether this was a branch checkout (1) or file checkout (0).
 check_post_checkout () {
 	test "$#" = 4 || BUG "check_post_checkout takes 4 args"
-	read old new flag <"$1" &&
-	test "$old" = "$2" && test "$new" = "$3" && test "$flag" = "$4"
+	echo "old=$2 new=$3 flag=$4" >expect &&
+	test_cmp expect "$1"
 }
 
 test_expect_success setup '
 	test_hook --setup post-checkout <<-\EOF &&
-	echo "$@" >.git/post-checkout.args
+	echo "old=$1 new=$2 flag=$3" >.git/post-checkout.args
 	EOF
 	test_commit one &&
 	test_commit two &&
@@ -112,7 +112,7 @@ test_rebase --merge
 test_expect_success 'post-checkout hook is triggered by clone' '
 	mkdir -p templates/hooks &&
 	write_script templates/hooks/post-checkout <<-\EOF &&
-	echo "$@" >"$GIT_DIR/post-checkout.args"
+	echo "old=$1 new=$2 flag=$3" >"$GIT_DIR/post-checkout.args"
 	EOF
 	git clone --template=templates . clone3 &&
 	check_post_checkout clone3/.git/post-checkout.args \
-- 
2.52.0.230.gd8af7cadaa