Re: [PATCH v4 28/36] hook tests: test for exact "pre-push" hook input
From: Emily Shaffer <hidden>
Date: 2021-08-20 00:16:41
On Tue, Aug 03, 2021 at 09:38:54PM +0200, Ævar Arnfjörð Bjarmason wrote:
Extend the tests added in ec55559f937 (push: Add support for pre-push hooks, 2013-01-13) to exhaustively test for the exact input we're expecting. This helps a parallel series that's refactoring how the hook is called, to e.g. make sure that we don't miss a trailing newline
The reference to "a parallel series" I think didn't belong in the commit-msg in the first place, and doesn't make sense now (because this is in said series, right?).
quoted hunk ↗ jump to hunk
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- t/t5571-pre-push-hook.sh | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh index ad8d5804f7b..d2857a6fbc0 100755 --- a/t/t5571-pre-push-hook.sh +++ b/t/t5571-pre-push-hook.sh@@ -11,7 +11,7 @@ HOOKDIR="$(git rev-parse --git-dir)/hooks" HOOK="$HOOKDIR/pre-push" mkdir -p "$HOOKDIR" write_script "$HOOK" <<EOF -cat >/dev/null +cat >actual exit 0 EOF@@ -20,10 +20,16 @@ test_expect_success 'setup' ' git init --bare repo1 && git remote add parent1 repo1 && test_commit one && - git push parent1 HEAD:foreign + cat >expect <<-EOF && + HEAD $(git rev-parse HEAD) refs/heads/foreign $(test_oid zero) + EOF + + test_when_finished "rm actual" && + git push parent1 HEAD:foreign && + test_cmp expect actual ' write_script "$HOOK" <<EOF -cat >/dev/null +cat >actual
Hm, I am not sure I like this. It upsets the usual convention of what
'actual' means ("output I captured from a 'git' command"). It is
nitpicky, but I would be happier to see it cat to some other filename,
like 'received-input'.
quoted hunk ↗ jump to hunk
exit 1 EOF@@ -32,11 +38,18 @@ export COMMIT1 test_expect_success 'push with failing hook' ' test_commit two && - test_must_fail git push parent1 HEAD + cat >expect <<-EOF && + HEAD $(git rev-parse HEAD) refs/heads/main $(test_oid zero) + EOF + + test_when_finished "rm actual" && + test_must_fail git push parent1 HEAD && + test_cmp expect actual ' test_expect_success '--no-verify bypasses hook' ' - git push --no-verify parent1 HEAD + git push --no-verify parent1 HEAD && + test_path_is_missing actual
Other than the naming nit, though, the test changes look reasonable to me to ensure we don't goof up the stdin support in hook.c. Thanks. With (or, I guess, without) changes, Reviewed-by: Emily Shaffer <redacted>
' COMMIT2="$(git rev-parse HEAD)" -- 2.33.0.rc0.595.ge31e012651d