| Message ID | patch-v4-28.36-ecf75f33233-20210803T191505Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Run hooks via "git run hook" & hook library | expand |
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?). > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > 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'. > 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 <emilyshaffer@google.com> > ' > > COMMIT2="$(git rev-parse HEAD)" > -- > 2.33.0.rc0.595.ge31e012651d >
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 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 ' COMMIT2="$(git rev-parse HEAD)"
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. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t5571-pre-push-hook.sh | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)