[v4,1/4] t7503: verify proper hook execution

Commit Message

Josh Steadmon Aug. 5, 2019, 10:43 p.m. UTC

t7503 did not verify that the expected hooks actually ran during
testing. Fix that by making the hook scripts write their $0 into a file
so that we can compare actual execution vs. expected execution.

While we're at it, do some test style cleanups, such as using
write_script() and doing setup inside a test_expect_success block.

Improved-by: Martin Ågren <[e-mail]>
Signed-off-by: Martin Ågren <[e-mail]>
Signed-off-by: Josh Steadmon <[e-mail]>
---
 t/t7503-pre-commit-hook.sh | 157 +++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 68 deletions(-)

Comments

Junio C Hamano Aug. 6, 2019, 6:14 p.m. UTC | #1

Josh Steadmon <[e-mail]> writes:

> t7503 did not verify that the expected hooks actually ran during
> testing. Fix that by making the hook scripts write their $0 into a file
> so that we can compare actual execution vs. expected execution.
>
> While we're at it, do some test style cleanups, such as using
> write_script() and doing setup inside a test_expect_success block.
> ...

The result mostly looks more pleasant compared to the original,
modulo a few nits.

>  test_expect_success 'with succeeding hook' '
> -
> -	echo "more" >> file &&
> +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
> +	ln -s "success.sample" "$PRECOMMIT" &&

Not all systems that want to run Git are capable of symbolic links.
We should be able to do a copy here, perhaps?  Just the most basic
form of "cp" without even "-p" would be sufficient as the target is
expected to be missing (i.e. "cp" or "ln -s" would be creating the
"$PRECOMMIT"), right?

Josh Steadmon Aug. 7, 2019, 6:11 p.m. UTC | #2

On 2019.08.06 11:14, Junio C Hamano wrote:
> Josh Steadmon <[e-mail]> writes:
> 
> > t7503 did not verify that the expected hooks actually ran during
> > testing. Fix that by making the hook scripts write their $0 into a file
> > so that we can compare actual execution vs. expected execution.
> >
> > While we're at it, do some test style cleanups, such as using
> > write_script() and doing setup inside a test_expect_success block.
> > ...
> 
> The result mostly looks more pleasant compared to the original,
> modulo a few nits.
> 
> >  test_expect_success 'with succeeding hook' '
> > -
> > -	echo "more" >> file &&
> > +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
> > +	ln -s "success.sample" "$PRECOMMIT" &&
> 
> Not all systems that want to run Git are capable of symbolic links.
> We should be able to do a copy here, perhaps?  Just the most basic
> form of "cp" without even "-p" would be sufficient as the target is
> expected to be missing (i.e. "cp" or "ln -s" would be creating the
> "$PRECOMMIT"), right?

Yeah, "cp" works here. Fixed in V5.

Patch

diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh
index 984889b39d..a71ec31222 100755
--- a/t/t7503-pre-commit-hook.sh
+++ b/t/t7503-pre-commit-hook.sh
@@ -4,124 +4,144 @@  test_description='pre-commit hook'
 
 . ./test-lib.sh
 
-test_expect_success 'with no hook' '
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+PRECOMMIT="$HOOKDIR/pre-commit"
+
+# Prepare sample scripts that write their $0 to actual_hooks
+test_expect_success 'sample script setup' '
+	mkdir -p "$HOOKDIR" &&
+	write_script "$HOOKDIR/success.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 0
+	EOF
+	write_script "$HOOKDIR/fail.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	write_script "$HOOKDIR/non-exec.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	chmod -x "$HOOKDIR/non-exec.sample" &&
+	write_script "$HOOKDIR/require-prefix.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	test $GIT_PREFIX = "success/"
+	EOF
+	write_script "$HOOKDIR/check-author.sample" <<-\EOF
+	echo $0 >>actual_hooks
+	test "$GIT_AUTHOR_NAME" = "New Author" &&
+	test "$GIT_AUTHOR_EMAIL" = "[e-mail]"
+	EOF
+'
 
-	echo "foo" > file &&
+test_expect_success 'with no hook' '
+	test_when_finished "rm -f actual_hooks" &&
+	echo "foo" >file &&
 	git add file &&
-	git commit -m "first"
-
+	git commit -m "first" &&
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success '--no-verify with no hook' '
-
-	echo "bar" > file &&
+	test_when_finished "rm -f actual_hooks" &&
+	echo "bar" >file &&
 	git add file &&
-	git commit --no-verify -m "bar"
-
+	git commit --no-verify -m "bar" &&
+	test_path_is_missing actual_hooks
 '
 
-# now install hook that always succeeds
-HOOKDIR="$(git rev-parse --git-dir)/hooks"
-HOOK="$HOOKDIR/pre-commit"
-mkdir -p "$HOOKDIR"
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 0
-EOF
-chmod +x "$HOOK"
-
 test_expect_success 'with succeeding hook' '
-
-	echo "more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more" >>file &&
 	git add file &&
-	git commit -m "more"
-
+	git commit -m "more" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with succeeding hook' '
-
-	echo "even more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+	echo "even more" >>file &&
 	git add file &&
-	git commit --no-verify -m "even more"
-
+	git commit --no-verify -m "even more" &&
+	test_path_is_missing actual_hooks
 '
 
-# now a hook that fails
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 1
-EOF
-
 test_expect_success 'with failing hook' '
-
-	echo "another" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "another" >>file &&
 	git add file &&
-	test_must_fail git commit -m "another"
-
+	test_must_fail git commit -m "another" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with failing hook' '
-
-	echo "stuff" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	echo "stuff" >>file &&
 	git add file &&
-	git commit --no-verify -m "stuff"
-
+	git commit --no-verify -m "stuff" &&
+	test_path_is_missing actual_hooks
 '
 
-chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
-
-	echo "content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "non-exec.sample" "$PRECOMMIT" &&
+	echo "content" >>file &&
 	git add file &&
-	git commit -m "content"
-
+	git commit -m "content" &&
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success POSIXPERM '--no-verify with non-executable hook' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "non-exec.sample" "$PRECOMMIT" &&
+	echo "more content" >>file &&
 	git add file &&
-	git commit --no-verify -m "more content"
-
+	git commit --no-verify -m "more content" &&
+	test_path_is_missing actual_hooks
 '
-chmod +x "$HOOK"
-
-# a hook that checks $GIT_PREFIX and succeeds inside the
-# success/ subdirectory only
-cat > "$HOOK" <<EOF
-#!/bin/sh
-test \$GIT_PREFIX = success/
-EOF
 
 test_expect_success 'with hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir success &&
 	(
 		cd success &&
 		git commit -m "hook requires GIT_PREFIX = success/"
 	) &&
-	rmdir success
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'with failing hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks fail" &&
+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir fail &&
 	(
 		cd fail &&
 		test_must_fail git commit -m "hook must fail"
 	) &&
-	rmdir fail &&
-	git checkout -- file
+	git checkout -- file &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'check the author in hook' '
-	write_script "$HOOK" <<-\EOF &&
-	test "$GIT_AUTHOR_NAME" = "New Author" &&
-	test "$GIT_AUTHOR_EMAIL" = "[e-mail]"
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "check-author.sample" "$PRECOMMIT" &&
+	cat >expected_hooks <<-EOF &&
+	$PRECOMMIT
+	$PRECOMMIT
+	$PRECOMMIT
 	EOF
 	test_must_fail git commit --allow-empty -m "by a.u.thor" &&
 	(
@@ -133,7 +153,8 @@  test_expect_success 'check the author in hook' '
 	) &&
 	git commit --author="New Author <[e-mail]>" \
 		--allow-empty -m "by new.author via command line" &&
-	git show -s
+	git show -s &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_done